Why Me In This Terrible Condition.
Greetings My Dear, I sent this mail praying it will found you in a good condition of health, since I myself are in a very critical health condition in which I sleep every night without knowing if I may be alive to see the next day. I am Mrs. Francisca Carlsen from Denmark wife of late Mr John Carlsen, a widow suffering from long time illness. I have some funds I inherited from my late husband, the sum of (eleven million dollars) my Doctor told me recently that I have serious sickness which is cancer problem. What disturbs me most is my stroke sickness. Having known my condition, I decided to donate this fund to a good person that will utilize it the way i am going to instruct herein. I need a very honest and God fearing person who can claim this money and use it for Charity works, for orphanages, widows and also build schools for less privileges that will be named after my late husband if possible and to promote the word of God and the effort that the house of God is maintained. I do not want a situation where this money will be used in an ungodly manner. That's why I'm taking this decision. I'm not afraid of death, so I know where I'm going. I accept this decision because I do not have any child who will inherit this money after I die. Please I want your sincerely and urgent answer to know if you will be able to execute this project, and I will give you more information on how the fund will be transferred to your bank account. I am waiting for your reply. May God Bless you, Mrs. Francisca Carlsen
HELLO
Did you get my previous Email??
[PATCH v6 1/1] http: add support selecting http version
From: Force Charlie
Usually we don't need to set libcurl to choose which version of the
HTTP protocol to use to communicate with a server.
But different versions of libcurl, the default value is not the same.
CURL >= 7.62.0: CURL_HTTP_VERSION_2TLS
CURL < 7.62: CURL_HTTP_VERSION_1_1
In order to give users the freedom to control the HTTP version,
we need to add a setting to choose which HTTP version to use.
Signed-off-by: Force Charlie
---
Documentation/config.txt | 9 +
http.c | 38 ++
2 files changed, 47 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 41a9ff2b6a..f397942128 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1935,6 +1935,15 @@ http.saveCookies::
If set, store cookies received during requests to the file specified by
http.cookieFile. Has no effect if http.cookieFile is unset.
+http.version::
+ Use the specified HTTP protocol version when communicating with a
server.
+ If you want to force the default. The available and default version
depend
+ on libcurl. Actually the possible values of
+ this option are:
+
+ - HTTP/2
+ - HTTP/1.1
+
http.sslVersion::
The SSL version to use when negotiating an SSL connection, if you
want to force the default. The available and default version
diff --git a/http.c b/http.c
index 3dc8c560d6..d6f3c4ee80 100644
--- a/http.c
+++ b/http.c
@@ -48,6 +48,7 @@ char curl_errorstr[CURL_ERROR_SIZE];
static int curl_ssl_verify = -1;
static int curl_ssl_try;
+static const char *curl_http_version = NULL;
static const char *ssl_cert;
static const char *ssl_cipherlist;
static const char *ssl_version;
@@ -284,6 +285,9 @@ static void process_curl_messages(void)
static int http_options(const char *var, const char *value, void *cb)
{
+ if (!strcmp("http.version",var)) {
+ return git_config_string(_http_version, var, value);
+ }
if (!strcmp("http.sslverify", var)) {
curl_ssl_verify = git_config_bool(var, value);
return 0;
@@ -789,6 +793,30 @@ static long get_curl_allowed_protocols(int from_user)
}
#endif
+#if LIBCURL_VERSION_NUM >=0x072f00
+static int get_curl_http_version_opt(const char *version_string, long *opt)
+{
+ int i;
+ static struct {
+ const char *name;
+ long opt_token;
+ } choice[] = {
+ { "HTTP/1.1", CURL_HTTP_VERSION_1_1 },
+ { "HTTP/2", CURL_HTTP_VERSION_2 }
+ };
+
+ for (i = 0; i < ARRAY_SIZE(choice); i++) {
+ if (!strcmp(version_string, choice[i].name)) {
+ *opt = choice[i].opt_token;
+ return 0;
+ }
+ }
+
+ return -1; /* not found */
+}
+
+#endif
+
static CURL *get_curl_handle(void)
{
CURL *result = curl_easy_init();
@@ -806,6 +834,16 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_SSL_VERIFYHOST, 2);
}
+#if LIBCURL_VERSION_NUM >= 0x072f00 // 7.47.0
+if (curl_http_version) {
+ long opt;
+ if (!get_curl_http_version_opt(curl_http_version, )) {
+ /* Set request use http version */
+ curl_easy_setopt(result, CURLOPT_HTTP_VERSION,opt);
+ }
+}
+#endif
+
#if LIBCURL_VERSION_NUM >= 0x070907
curl_easy_setopt(result, CURLOPT_NETRC, CURL_NETRC_OPTIONAL);
#endif
--
gitgitgadget
[PATCH v6 0/1] http: add support selecting http version
Usually we don't need to set libcurl to choose which version of the HTTP protocol to use to communicate with a server. But different versions of libcurl, the default value is not the same. CURL >= 7.62.0: CURL_HTTP_VERSION_2TLS CURL < 7.62: CURL_HTTP_VERSION_1_1 In order to give users the freedom to control the HTTP version, we need to add a setting to choose which HTTP version to use. This patch support force enable HTTP/2 or HTTP/1.1. example: GIT_CURL_VERBOSE=1 git2 -c http.version=HTTP/2 ls-remote https://bitbucket.org/aquariusjay/deeplab-public-ver2.git Force Charlie (1): http: add support selecting http version Documentation/config.txt | 9 + http.c | 38 ++ 2 files changed, 47 insertions(+) base-commit: 8858448bb49332d353febc078ce4a3abcc962efe Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-69%2Ffcharlie%2Fmaster-v6 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-69/fcharlie/master-v6 Pull-Request: https://github.com/gitgitgadget/git/pull/69 Range-diff vs v5: 1: cdd93048ba ! 1: 93fda67198 http: add support selecting http version @@ -2,8 +2,38 @@ http: add support selecting http version +Usually we don't need to set libcurl to choose which version of the +HTTP protocol to use to communicate with a server. +But different versions of libcurl, the default value is not the same. + +CURL >= 7.62.0: CURL_HTTP_VERSION_2TLS +CURL < 7.62: CURL_HTTP_VERSION_1_1 + +In order to give users the freedom to control the HTTP version, +we need to add a setting to choose which HTTP version to use. + Signed-off-by: Force Charlie +diff --git a/Documentation/config.txt b/Documentation/config.txt +--- a/Documentation/config.txt b/Documentation/config.txt +@@ + If set, store cookies received during requests to the file specified by + http.cookieFile. Has no effect if http.cookieFile is unset. + ++http.version:: ++ Use the specified HTTP protocol version when communicating with a server. ++ If you want to force the default. The available and default version depend ++ on libcurl. Actually the possible values of ++ this option are: ++ ++ - HTTP/2 ++ - HTTP/1.1 ++ + http.sslVersion:: + The SSL version to use when negotiating an SSL connection, if you + want to force the default. The available and default version + diff --git a/http.c b/http.c --- a/http.c +++ b/http.c -- gitgitgadget
Re: [PATCH v2 0/2] Reimplement rebase --merge via interactive machinery
On Wed, Nov 7, 2018 at 10:02 PM Elijah Newren wrote: > > Now that the rewrite-interactive-rebases-in-C series have finally > merged to master, this series deletes git-rebase--merge.sh and > reimplements the --merge behavior on top of the interactive machinery. > > Differences since v1: > - Updated code to hook into builtin/rebase.C instead of git-rebase.sh > > (No range-diff provided, because it has been months since v1, and v1 > was only RFC and was only discussed at a high level.) Actually, that's not correct; it's been so long that I forgot. Dscho and Phillip both reviewed it and I updated my series at the time with their suggestions, but didn't re-submit because it depended on so many other series and conflicted with the rebase-in-C work. So other differences include the changes I made to address their feedback, but...even if I dug up the old v1 and created a range-diff against it I'm not so sure it'd be helpful. If anyone thinks it would be, holler and I'll generate it. Original series here: https://public-inbox.org/git/20180607171344.23331-1-new...@gmail.com/
[PATCH v5 0/1] http: add support selecting http version
Normally, git doesn't need to set curl to select the HTTP version, it works
fine without HTTP/2. Adding HTTP/2 support is a icing on the cake.
This patch support force enable HTTP/2 or HTTP/1.1.
example:
GIT_CURL_VERBOSE=1 git2 -c http.version=HTTP/2 ls-remote
https://bitbucket.org/aquariusjay/deeplab-public-ver2.git
Force Charlie (1):
http: add support selecting http version
http.c | 38 ++
1 file changed, 38 insertions(+)
base-commit: 8858448bb49332d353febc078ce4a3abcc962efe
Published-As:
https://github.com/gitgitgadget/git/releases/tags/pr-69%2Ffcharlie%2Fmaster-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git
pr-69/fcharlie/master-v5
Pull-Request: https://github.com/gitgitgadget/git/pull/69
Range-diff vs v4:
1: 4f5a935c43 < -: -- http: add support selecting http version
2: 06e9685d2b < -: -- support force use http 1.1
3: eee67d8356 < -: -- fix curl version to support
CURL_HTTP_VERSION_2TLS
4: 0a7794722b ! 1: cdd93048ba http: change http.version value type
@@ -1,6 +1,6 @@
Author: Force Charlie
-http: change http.version value type
+http: add support selecting http version
Signed-off-by: Force Charlie
@@ -11,21 +11,20 @@
static int curl_ssl_verify = -1;
static int curl_ssl_try;
--static int curl_http_version = 0;
+static const char *curl_http_version = NULL;
static const char *ssl_cert;
static const char *ssl_cipherlist;
static const char *ssl_version;
@@
+
static int http_options(const char *var, const char *value, void *cb)
{
- if (!strcmp("http.version",var)) {
-- curl_http_version=git_config_int(var,value);
-- return 0;
++ if (!strcmp("http.version",var)) {
+ return git_config_string(_http_version, var, value);
- }
++ }
if (!strcmp("http.sslverify", var)) {
curl_ssl_verify = git_config_bool(var, value);
+ return 0;
@@
}
#endif
@@ -58,21 +57,19 @@
{
CURL *result = curl_easy_init();
@@
+ curl_easy_setopt(result, CURLOPT_SSL_VERIFYHOST, 2);
}
- #if LIBCURL_VERSION_NUM >= 0x072f00 // 7.47.0
--// curl_http_version 0 is default.
--if (curl_http_version == 20) {
-- /* Enable HTTP2*/
-- curl_easy_setopt(result,
CURLOPT_HTTP_VERSION,CURL_HTTP_VERSION_2TLS);
--} else if (curl_http_version == 11) {
-- curl_easy_setopt(result,
CURLOPT_HTTP_VERSION,CURL_HTTP_VERSION_1_1);
++#if LIBCURL_VERSION_NUM >= 0x072f00 // 7.47.0
+if (curl_http_version) {
+ long opt;
+ if (!get_curl_http_version_opt(curl_http_version, )) {
+ /* Set request use http version */
+ curl_easy_setopt(result, CURLOPT_HTTP_VERSION,opt);
+ }
- }
++}
++#endif
++
+ #if LIBCURL_VERSION_NUM >= 0x070907
+ curl_easy_setopt(result, CURLOPT_NETRC, CURL_NETRC_OPTIONAL);
#endif
-
--
gitgitgadget
[PATCH v5 1/1] http: add support selecting http version
From: Force Charlie
Signed-off-by: Force Charlie
---
http.c | 38 ++
1 file changed, 38 insertions(+)
diff --git a/http.c b/http.c
index 3dc8c560d6..d6f3c4ee80 100644
--- a/http.c
+++ b/http.c
@@ -48,6 +48,7 @@ char curl_errorstr[CURL_ERROR_SIZE];
static int curl_ssl_verify = -1;
static int curl_ssl_try;
+static const char *curl_http_version = NULL;
static const char *ssl_cert;
static const char *ssl_cipherlist;
static const char *ssl_version;
@@ -284,6 +285,9 @@ static void process_curl_messages(void)
static int http_options(const char *var, const char *value, void *cb)
{
+ if (!strcmp("http.version",var)) {
+ return git_config_string(_http_version, var, value);
+ }
if (!strcmp("http.sslverify", var)) {
curl_ssl_verify = git_config_bool(var, value);
return 0;
@@ -789,6 +793,30 @@ static long get_curl_allowed_protocols(int from_user)
}
#endif
+#if LIBCURL_VERSION_NUM >=0x072f00
+static int get_curl_http_version_opt(const char *version_string, long *opt)
+{
+ int i;
+ static struct {
+ const char *name;
+ long opt_token;
+ } choice[] = {
+ { "HTTP/1.1", CURL_HTTP_VERSION_1_1 },
+ { "HTTP/2", CURL_HTTP_VERSION_2 }
+ };
+
+ for (i = 0; i < ARRAY_SIZE(choice); i++) {
+ if (!strcmp(version_string, choice[i].name)) {
+ *opt = choice[i].opt_token;
+ return 0;
+ }
+ }
+
+ return -1; /* not found */
+}
+
+#endif
+
static CURL *get_curl_handle(void)
{
CURL *result = curl_easy_init();
@@ -806,6 +834,16 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_SSL_VERIFYHOST, 2);
}
+#if LIBCURL_VERSION_NUM >= 0x072f00 // 7.47.0
+if (curl_http_version) {
+ long opt;
+ if (!get_curl_http_version_opt(curl_http_version, )) {
+ /* Set request use http version */
+ curl_easy_setopt(result, CURLOPT_HTTP_VERSION,opt);
+ }
+}
+#endif
+
#if LIBCURL_VERSION_NUM >= 0x070907
curl_easy_setopt(result, CURLOPT_NETRC, CURL_NETRC_OPTIONAL);
#endif
--
gitgitgadget
[PATCH v4 2/4] support force use http 1.1
From: Force Charlie
Signed-off-by: Force Charlie
---
http.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/http.c b/http.c
index 99cb04faba..b2ec31aef5 100644
--- a/http.c
+++ b/http.c
@@ -48,7 +48,7 @@ char curl_errorstr[CURL_ERROR_SIZE];
static int curl_ssl_verify = -1;
static int curl_ssl_try;
-static int curl_http_version = 11;
+static int curl_http_version = 0;
static const char *ssl_cert;
static const char *ssl_cipherlist;
static const char *ssl_version;
@@ -811,11 +811,14 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_SSL_VERIFYHOST, 2);
}
-#if LIBCURL_VERSION_NUM >= 0x073100
- if(curl_http_version == 20){
- /* CURL Enable HTTP2*/
- curl_easy_setopt(result,
CURLOPT_HTTP_VERSION,CURL_HTTP_VERSION_2);
- }
+#if LIBCURL_VERSION_NUM >= 0x074700
+// curl_http_version 0 is default.
+if (curl_http_version == 20) {
+ /* Enable HTTP2 when request TLS*/
+ curl_easy_setopt(result,
CURLOPT_HTTP_VERSION,CURL_HTTP_VERSION_2TLS);
+} else if (curl_http_version == 11) {
+ curl_easy_setopt(result,
CURLOPT_HTTP_VERSION,CURL_HTTP_VERSION_1_1);
+}
#endif
#if LIBCURL_VERSION_NUM >= 0x070907
--
gitgitgadget
Re: [PATCH v3 0/4] http: add support selecting http version
"Force.Charlie-I via GitGitGadget" writes: > Normally, git doesn't need to set curl to select the HTTP version, it works > fine without HTTP/2. Adding HTTP/2 support is a icing on the cake. > > This patch support force enable HTTP/2 or HTTP/1.1. > > example: > > GIT_CURL_VERBOSE=1 git2 -c http.version=HTTP/2 ls-remote > https://bitbucket.org/aquariusjay/deeplab-public-ver2.git > > Force Charlie (4): > http: add support selecting http version > support force use http 1.1 > fix curl version to support CURL_HTTP_VERSION_2TLS > http: change http.version value type When somebody reads over these four patches as a first-time reader, I think s/he notices a couple of things: - In the proposed log messages, there is no explanation on the reason why we are doing these changes. - Each of the steps n/4 (n > 1) looks more like "oops, it was a mistake that we did not do this in earlier patch, and here is to correct that". - There is no test or documentation. I suspect that a single patch that updates http.c, Documentation/ and t/ at the same time should be sufficient for a change of this size. Thanks. > http.c | 36 > 1 file changed, 36 insertions(+) > > > base-commit: 8858448bb49332d353febc078ce4a3abcc962efe > Published-As: > https://github.com/gitgitgadget/git/releases/tags/pr-69%2Ffcharlie%2Fmaster-v3 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git > pr-69/fcharlie/master-v3 > Pull-Request: https://github.com/gitgitgadget/git/pull/69 > > Range-diff vs v2: > > 1: 4f5a935c43 = 1: 4f5a935c43 http: add support selecting http version > 2: 06e9685d2b = 2: 06e9685d2b support force use http 1.1 > 3: eee67d8356 = 3: eee67d8356 fix curl version to support > CURL_HTTP_VERSION_2TLS > -: -- > 4: ef975b6093 http: change http.version value type
[PATCH v4 0/4] http: add support selecting http version
Normally, git doesn't need to set curl to select the HTTP version, it works
fine without HTTP/2. Adding HTTP/2 support is a icing on the cake.
This patch support force enable HTTP/2 or HTTP/1.1.
example:
GIT_CURL_VERBOSE=1 git2 -c http.version=HTTP/2 ls-remote
https://bitbucket.org/aquariusjay/deeplab-public-ver2.git
Force Charlie (4):
http: add support selecting http version
support force use http 1.1
fix curl version to support CURL_HTTP_VERSION_2TLS
http: change http.version value type
http.c | 38 ++
1 file changed, 38 insertions(+)
base-commit: 8858448bb49332d353febc078ce4a3abcc962efe
Published-As:
https://github.com/gitgitgadget/git/releases/tags/pr-69%2Ffcharlie%2Fmaster-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git
pr-69/fcharlie/master-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/69
Range-diff vs v3:
1: 4f5a935c43 = 1: 4f5a935c43 http: add support selecting http version
2: 06e9685d2b = 2: 06e9685d2b support force use http 1.1
3: eee67d8356 = 3: eee67d8356 fix curl version to support
CURL_HTTP_VERSION_2TLS
4: ef975b6093 ! 4: 0a7794722b http: change http.version value type
@@ -67,10 +67,12 @@
- curl_easy_setopt(result,
CURLOPT_HTTP_VERSION,CURL_HTTP_VERSION_2TLS);
-} else if (curl_http_version == 11) {
- curl_easy_setopt(result,
CURLOPT_HTTP_VERSION,CURL_HTTP_VERSION_1_1);
-+long opt=-1;
-+if (curl_http_version
&&!get_curl_http_version_opt(curl_http_version, )) {
-+ /* Set request use http version */
-+ curl_easy_setopt(result, CURLOPT_HTTP_VERSION,opt);
++if (curl_http_version) {
++ long opt;
++ if (!get_curl_http_version_opt(curl_http_version, )) {
++ /* Set request use http version */
++ curl_easy_setopt(result, CURLOPT_HTTP_VERSION,opt);
++ }
}
#endif
--
gitgitgadget
[PATCH v4 4/4] http: change http.version value type
From: Force Charlie
Signed-off-by: Force Charlie
---
http.c | 41 -
1 file changed, 32 insertions(+), 9 deletions(-)
diff --git a/http.c b/http.c
index 86e454cff5..d6f3c4ee80 100644
--- a/http.c
+++ b/http.c
@@ -48,7 +48,7 @@ char curl_errorstr[CURL_ERROR_SIZE];
static int curl_ssl_verify = -1;
static int curl_ssl_try;
-static int curl_http_version = 0;
+static const char *curl_http_version = NULL;
static const char *ssl_cert;
static const char *ssl_cipherlist;
static const char *ssl_version;
@@ -286,8 +286,7 @@ static void process_curl_messages(void)
static int http_options(const char *var, const char *value, void *cb)
{
if (!strcmp("http.version",var)) {
- curl_http_version=git_config_int(var,value);
- return 0;
+ return git_config_string(_http_version, var, value);
}
if (!strcmp("http.sslverify", var)) {
curl_ssl_verify = git_config_bool(var, value);
@@ -794,6 +793,30 @@ static long get_curl_allowed_protocols(int from_user)
}
#endif
+#if LIBCURL_VERSION_NUM >=0x072f00
+static int get_curl_http_version_opt(const char *version_string, long *opt)
+{
+ int i;
+ static struct {
+ const char *name;
+ long opt_token;
+ } choice[] = {
+ { "HTTP/1.1", CURL_HTTP_VERSION_1_1 },
+ { "HTTP/2", CURL_HTTP_VERSION_2 }
+ };
+
+ for (i = 0; i < ARRAY_SIZE(choice); i++) {
+ if (!strcmp(version_string, choice[i].name)) {
+ *opt = choice[i].opt_token;
+ return 0;
+ }
+ }
+
+ return -1; /* not found */
+}
+
+#endif
+
static CURL *get_curl_handle(void)
{
CURL *result = curl_easy_init();
@@ -812,12 +835,12 @@ static CURL *get_curl_handle(void)
}
#if LIBCURL_VERSION_NUM >= 0x072f00 // 7.47.0
-// curl_http_version 0 is default.
-if (curl_http_version == 20) {
- /* Enable HTTP2*/
- curl_easy_setopt(result,
CURLOPT_HTTP_VERSION,CURL_HTTP_VERSION_2TLS);
-} else if (curl_http_version == 11) {
- curl_easy_setopt(result,
CURLOPT_HTTP_VERSION,CURL_HTTP_VERSION_1_1);
+if (curl_http_version) {
+ long opt;
+ if (!get_curl_http_version_opt(curl_http_version, )) {
+ /* Set request use http version */
+ curl_easy_setopt(result, CURLOPT_HTTP_VERSION,opt);
+ }
}
#endif
--
gitgitgadget
[PATCH v4 3/4] fix curl version to support CURL_HTTP_VERSION_2TLS
From: Force Charlie
Signed-off-by: Force Charlie
---
http.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/http.c b/http.c
index b2ec31aef5..86e454cff5 100644
--- a/http.c
+++ b/http.c
@@ -811,10 +811,10 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_SSL_VERIFYHOST, 2);
}
-#if LIBCURL_VERSION_NUM >= 0x074700
+#if LIBCURL_VERSION_NUM >= 0x072f00 // 7.47.0
// curl_http_version 0 is default.
if (curl_http_version == 20) {
- /* Enable HTTP2 when request TLS*/
+ /* Enable HTTP2*/
curl_easy_setopt(result,
CURLOPT_HTTP_VERSION,CURL_HTTP_VERSION_2TLS);
} else if (curl_http_version == 11) {
curl_easy_setopt(result,
CURLOPT_HTTP_VERSION,CURL_HTTP_VERSION_1_1);
--
gitgitgadget
[PATCH v4 1/4] http: add support selecting http version
From: Force Charlie
Signed-off-by: Force Charlie
---
http.c | 12
1 file changed, 12 insertions(+)
diff --git a/http.c b/http.c
index 3dc8c560d6..99cb04faba 100644
--- a/http.c
+++ b/http.c
@@ -48,6 +48,7 @@ char curl_errorstr[CURL_ERROR_SIZE];
static int curl_ssl_verify = -1;
static int curl_ssl_try;
+static int curl_http_version = 11;
static const char *ssl_cert;
static const char *ssl_cipherlist;
static const char *ssl_version;
@@ -284,6 +285,10 @@ static void process_curl_messages(void)
static int http_options(const char *var, const char *value, void *cb)
{
+ if (!strcmp("http.version",var)) {
+ curl_http_version=git_config_int(var,value);
+ return 0;
+ }
if (!strcmp("http.sslverify", var)) {
curl_ssl_verify = git_config_bool(var, value);
return 0;
@@ -806,6 +811,13 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_SSL_VERIFYHOST, 2);
}
+#if LIBCURL_VERSION_NUM >= 0x073100
+ if(curl_http_version == 20){
+ /* CURL Enable HTTP2*/
+ curl_easy_setopt(result,
CURLOPT_HTTP_VERSION,CURL_HTTP_VERSION_2);
+ }
+#endif
+
#if LIBCURL_VERSION_NUM >= 0x070907
curl_easy_setopt(result, CURLOPT_NETRC, CURL_NETRC_OPTIONAL);
#endif
--
gitgitgadget
Re: What's cooking in git.git (Nov 2018, #03; Wed, 7)
Junio C Hamano writes: >> Not surprising. The i18n series makes fsck output localized strings >> and without updating grep to test_i18ngrep, new tests will fail. If >> 'pu' was passing before, I'm ok with just ejecting this series for >> now. Then I wait for the other to land, rebase, fixup and resubmit. > > Let me first see if I can come up with a merge-fix that can be > carried around during the time this topic cooks, before talking > about dropping and reattempting the series. > > For a change like this, a time-window in which the codebase is > quiescent enough may never come, and because the changes go all over > the place, mostly but not entirely repetitive, it costs a lot, not > just to write but also to review them. I have configured the machinery used to rebuild integration branches so that the following is squashed in when the nd/i18n topic is merged. The part that touches t1450 needs to be split out and squashed when nd/per-worktree-ref-iteration gets merged, *if* the nd/i18n topic needs to jump over the other topic, but I'll worry about it when it happens---I think per-worktree ref iteration series that is in next should be ready enough that we do not mind having nd/i18n waiting for it. Anyway, here is what is in refs/merge-fix/nd/i18n, which gets cherry-picked and squashed into the merge when nd/i18n is merged. t/t1450-fsck.sh| 8 t/t5616-partial-clone.sh | 2 +- t/t5703-upload-pack-ref-in-want.sh | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh index 8a5f4c7189..2e5e979336 100755 --- a/t/t1450-fsck.sh +++ b/t/t1450-fsck.sh @@ -109,7 +109,7 @@ test_expect_success 'HEAD link pointing at a funny object (from different wt)' ' echo $ZERO_OID >.git/HEAD && # avoid corrupt/broken HEAD from interfering with repo discovery test_must_fail git -C wt fsck 2>out && - grep "main-worktree/HEAD: detached HEAD points" out + test_i18ngrep "main-worktree/HEAD: detached HEAD points" out ' test_expect_success 'other worktree HEAD link pointing at a funny object' ' @@ -117,7 +117,7 @@ test_expect_success 'other worktree HEAD link pointing at a funny object' ' git worktree add other && echo $ZERO_OID >.git/worktrees/other/HEAD && test_must_fail git fsck 2>out && - grep "worktrees/other/HEAD: detached HEAD points" out + test_i18ngrep "worktrees/other/HEAD: detached HEAD points" out ' test_expect_success 'other worktree HEAD link pointing at missing object' ' @@ -125,7 +125,7 @@ test_expect_success 'other worktree HEAD link pointing at missing object' ' git worktree add other && echo "Contents missing from repo" | git hash-object --stdin >.git/worktrees/other/HEAD && test_must_fail git fsck 2>out && - grep "worktrees/other/HEAD: invalid sha1 pointer" out + test_i18ngrep "worktrees/other/HEAD: invalid sha1 pointer" out ' test_expect_success 'other worktree HEAD link pointing at a funny place' ' @@ -133,7 +133,7 @@ test_expect_success 'other worktree HEAD link pointing at a funny place' ' git worktree add other && echo "ref: refs/funny/place" >.git/worktrees/other/HEAD && test_must_fail git fsck 2>out && - grep "worktrees/other/HEAD points to something strange" out + test_i18ngrep "worktrees/other/HEAD points to something strange" out ' test_expect_success 'email without @ is okay' ' diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh index 336f02a41a..9643acb161 100755 --- a/t/t5616-partial-clone.sh +++ b/t/t5616-partial-clone.sh @@ -281,7 +281,7 @@ test_expect_success 'upon cloning, check that all refs point to objects' ' test_must_fail git -c protocol.version=2 clone \ --filter=blob:none $HTTPD_URL/one_time_sed/server repo 2>err && - grep "did not send all necessary objects" err && + test_i18ngrep "did not send all necessary objects" err && # Ensure that the one-time-sed script was used. ! test -e "$HTTPD_ROOT_PATH/one-time-sed" diff --git a/t/t5703-upload-pack-ref-in-want.sh b/t/t5703-upload-pack-ref-in-want.sh index 3f58f05cbb..7053899cb5 100755 --- a/t/t5703-upload-pack-ref-in-want.sh +++ b/t/t5703-upload-pack-ref-in-want.sh @@ -208,7 +208,7 @@ test_expect_success 'server is initially ahead - no ref in want' ' cp -r "$LOCAL_PRISTINE" local && inconsistency master 1234567890123456789012345678901234567890 && test_must_fail git -C local fetch 2>err && - grep "ERR upload-pack: not our ref" err + test_i18ngrep "ERR upload-pack: not our ref" err ' test_expect_success 'server is initially ahead - ref in want' ' @@ -254,7 +254,7 @@ test_expect_success 'server loses a ref - ref in want' ' echo "s/master/raster/" >"$HTTPD_ROOT_PATH/one-time-sed" && test_must_fail git -C local fetch 2>err && - grep "ERR unknown ref
[PATCH v2 0/2] Reimplement rebase --merge via interactive machinery
Now that the rewrite-interactive-rebases-in-C series have finally merged to master, this series deletes git-rebase--merge.sh and reimplements the --merge behavior on top of the interactive machinery. Differences since v1: - Updated code to hook into builtin/rebase.C instead of git-rebase.sh (No range-diff provided, because it has been months since v1, and v1 was only RFC and was only discussed at a high level.) Elijah Newren (2): git-rebase, sequencer: extend --quiet option for the interactive machinery rebase: Implement --merge via git-rebase--interactive .gitignore| 1 - Documentation/git-rebase.txt | 17 +--- Makefile | 1 - builtin/rebase.c | 15 ++- git-legacy-rebase.sh | 57 +-- git-rebase--common.sh | 2 +- git-rebase--merge.sh | 164 -- sequencer.c | 23 +++-- sequencer.h | 1 + t/t3406-rebase-message.sh | 7 +- t/t3420-rebase-autostash.sh | 78 ++ t/t3421-rebase-topology-linear.sh | 10 +- t/t3425-rebase-topology-merges.sh | 6 +- t/t5407-post-rewrite-hook.sh | 1 + t/t9903-bash-prompt.sh| 2 +- 15 files changed, 67 insertions(+), 318 deletions(-) delete mode 100644 git-rebase--merge.sh -- 2.19.1.858.g526e8fe740.dirty
[PATCH v2 2/2] rebase: Implement --merge via git-rebase--interactive
Interactive rebases are implemented in terms of cherry-pick rather than the merge-recursive builtin, but cherry-pick also calls into the recursive merge machinery by default and can accept special merge strategies and/or special strategy options. As such, there really is not any need for having both git-rebase--merge and git-rebase--interactive anymore. Delete git-rebase--merge.sh and have the --merge option be implemented by the now built-in interactive machinery. Note that this change fixes a few known test failures (see t3421). testcase modification notes: t3406: --interactive and --merge had slightly different progress output while running; adjust a test to match t3420: these test precise output while running, but rebase--am, rebase--merge, and rebase--interactive all were built on very different commands (am, merge-recursive, cherry-pick), so the tests expected different output for each type. Now we expect --merge and --interactive to have the same output. t3421: --interactive fixes some bugs in --merge! Wahoo! t3425: topology linearization was inconsistent across flavors of rebase, as already noted in a TODO comment in the testcase. This was not considered a bug before, so getting a different linearization due to switching out backends should not be considered a bug now. t5407: different rebase types varied slightly in how many times checkout or commit or equivalents were called based on a quick comparison of this tests and previous ones which covered different rebase flavors. I think this is just attributable to this difference. t9903: --merge uses the interactive backend so the prompt expected is now REBASE-i. Signed-off-by: Elijah Newren --- .gitignore| 1 - Documentation/git-rebase.txt | 17 +--- Makefile | 1 - builtin/rebase.c | 10 +- git-legacy-rebase.sh | 55 +- git-rebase--merge.sh | 164 -- t/t3406-rebase-message.sh | 7 +- t/t3420-rebase-autostash.sh | 78 ++ t/t3421-rebase-topology-linear.sh | 10 +- t/t3425-rebase-topology-merges.sh | 6 +- t/t5407-post-rewrite-hook.sh | 1 + t/t9903-bash-prompt.sh| 2 +- 12 files changed, 50 insertions(+), 302 deletions(-) delete mode 100644 git-rebase--merge.sh diff --git a/.gitignore b/.gitignore index 0d77ea5894..910b1d2d2f 100644 --- a/.gitignore +++ b/.gitignore @@ -124,7 +124,6 @@ /git-rebase--am /git-rebase--common /git-rebase--interactive -/git-rebase--merge /git-rebase--preserve-merges /git-receive-pack /git-reflog diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 3407d835bd..35084f5681 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -504,15 +504,7 @@ See also INCOMPATIBLE OPTIONS below. INCOMPATIBLE OPTIONS -git-rebase has many flags that are incompatible with each other, -predominantly due to the fact that it has three different underlying -implementations: - - * one based on linkgit:git-am[1] (the default) - * one based on git-merge-recursive (merge backend) - * one based on linkgit:git-cherry-pick[1] (interactive backend) - -Flags only understood by the am backend: +The following options: * --committer-date-is-author-date * --ignore-date @@ -520,15 +512,12 @@ Flags only understood by the am backend: * --ignore-whitespace * -C -Flags understood by both merge and interactive backends: +are incompatible with the following options: * --merge * --strategy * --strategy-option * --allow-empty-message - -Flags only understood by the interactive backend: - * --[no-]autosquash * --rebase-merges * --preserve-merges @@ -539,7 +528,7 @@ Flags only understood by the interactive backend: * --edit-todo * --root when used in combination with --onto -Other incompatible flag pairs: +In addition, the following pairs of options are incompatible: * --preserve-merges and --interactive * --preserve-merges and --signoff diff --git a/Makefile b/Makefile index bbfbb4292d..18e79fdea8 100644 --- a/Makefile +++ b/Makefile @@ -628,7 +628,6 @@ SCRIPT_LIB += git-parse-remote SCRIPT_LIB += git-rebase--am SCRIPT_LIB += git-rebase--common SCRIPT_LIB += git-rebase--preserve-merges -SCRIPT_LIB += git-rebase--merge SCRIPT_LIB += git-sh-setup SCRIPT_LIB += git-sh-i18n diff --git a/builtin/rebase.c b/builtin/rebase.c index be004406a6..d55bbb9eb9 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -118,7 +118,7 @@ static void imply_interactive(struct rebase_options *opts, const char *option) case REBASE_PRESERVE_MERGES: break; case REBASE_MERGE: - /* we silently *upgrade* --merge to --interactive if needed */ + /* we now implement --merge via --interactive */
[PATCH v2 1/2] git-rebase, sequencer: extend --quiet option for the interactive machinery
While 'quiet' and 'interactive' may sound like antonyms, the interactive
machinery actually has logic that implements several
interactive_rebase=implied cases (--exec, --keep-empty, --rebase-merges)
which won't pop up an editor. The rewrite of interactive rebase in C
added a quiet option, though it only turns stats off. Since we want to
make the interactive machinery also take over for git-rebase--merge, it
should fully implement the --quiet option.
git-rebase--interactive was already somewhat quieter than
git-rebase--merge and git-rebase--am, possibly because cherry-pick has
just traditionally been quieter. As such, we only drop a few
informational messages -- "Rebasing (n/m)" and "Succesfully rebased..."
Also, for simplicity, remove the differences in how quiet and verbose
options were recorded. Having one be signalled by the presence of a
"verbose" file in the state_dir, while the other was signalled by the
contents of a "quiet" file was just weirdly inconsistent. (This
inconsistency pre-dated the rewrite into C.) Make them consistent by
having them both key off the presence of the file.
Signed-off-by: Elijah Newren
---
builtin/rebase.c | 5 +
git-legacy-rebase.sh | 2 +-
git-rebase--common.sh | 2 +-
sequencer.c | 23 +--
sequencer.h | 1 +
5 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 0ee06aa363..be004406a6 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -181,10 +181,7 @@ static int read_basic_state(struct rebase_options *opts)
if (get_oid(buf.buf, >orig_head))
return error(_("invalid orig-head: '%s'"), buf.buf);
- strbuf_reset();
- if (read_one(state_dir_path("quiet", opts), ))
- return -1;
- if (buf.len)
+ if (file_exists(state_dir_path("quiet", opts)))
opts->flags &= ~REBASE_NO_QUIET;
else
opts->flags |= REBASE_NO_QUIET;
diff --git a/git-legacy-rebase.sh b/git-legacy-rebase.sh
index 75a08b2683..da27bfca5a 100755
--- a/git-legacy-rebase.sh
+++ b/git-legacy-rebase.sh
@@ -113,7 +113,7 @@ read_basic_state () {
else
orig_head=$(cat "$state_dir"/head)
fi &&
- GIT_QUIET=$(cat "$state_dir"/quiet) &&
+ test -f "$state_dir"/quiet && GIT_QUIET=t
test -f "$state_dir"/verbose && verbose=t
test -f "$state_dir"/strategy && strategy="$(cat "$state_dir"/strategy)"
test -f "$state_dir"/strategy_opts &&
diff --git a/git-rebase--common.sh b/git-rebase--common.sh
index 7e39d22871..dc18c682fa 100644
--- a/git-rebase--common.sh
+++ b/git-rebase--common.sh
@@ -10,7 +10,7 @@ write_basic_state () {
echo "$head_name" > "$state_dir"/head-name &&
echo "$onto" > "$state_dir"/onto &&
echo "$orig_head" > "$state_dir"/orig-head &&
- echo "$GIT_QUIET" > "$state_dir"/quiet &&
+ test t = "$GIT_QUIET" && : > "$state_dir"/quiet
test t = "$verbose" && : > "$state_dir"/verbose
test -n "$strategy" && echo "$strategy" > "$state_dir"/strategy
test -n "$strategy_opts" && echo "$strategy_opts" > \
diff --git a/sequencer.c b/sequencer.c
index 9e1ab3a2a7..bd8337dbf1 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -150,6 +150,7 @@ static GIT_PATH_FUNC(rebase_path_refs_to_delete,
"rebase-merge/refs-to-delete")
static GIT_PATH_FUNC(rebase_path_gpg_sign_opt, "rebase-merge/gpg_sign_opt")
static GIT_PATH_FUNC(rebase_path_orig_head, "rebase-merge/orig-head")
static GIT_PATH_FUNC(rebase_path_verbose, "rebase-merge/verbose")
+static GIT_PATH_FUNC(rebase_path_quiet, "rebase-merge/quiet")
static GIT_PATH_FUNC(rebase_path_signoff, "rebase-merge/signoff")
static GIT_PATH_FUNC(rebase_path_head_name, "rebase-merge/head-name")
static GIT_PATH_FUNC(rebase_path_onto, "rebase-merge/onto")
@@ -157,7 +158,6 @@ static GIT_PATH_FUNC(rebase_path_autostash,
"rebase-merge/autostash")
static GIT_PATH_FUNC(rebase_path_strategy, "rebase-merge/strategy")
static GIT_PATH_FUNC(rebase_path_strategy_opts, "rebase-merge/strategy_opts")
static GIT_PATH_FUNC(rebase_path_allow_rerere_autoupdate,
"rebase-merge/allow_rerere_autoupdate")
-static GIT_PATH_FUNC(rebase_path_quiet, "rebase-merge/quiet")
static int git_sequencer_config(const char *k, const char *v, void *cb)
{
@@ -2307,6 +2307,9 @@ static int read_populate_opts(struct replay_opts *opts)
if (file_exists(rebase_path_verbose()))
opts->verbose = 1;
+ if (file_exists(rebase_path_quiet()))
+ opts->quiet = 1;
+
if (file_exists(rebase_path_signoff())) {
opts->allow_ff = 0;
opts->signoff = 1;
@@ -2374,9 +2377,6 @@ int write_basic_state(struct replay_opts *opts, const
char *head_name,
if (quiet)
write_file(rebase_path_quiet(), "%s\n", quiet);
- else
-
Re: [PATCH v5 10/10] merge-recursive: combine error handling
Elijah Newren writes: > From: Derrick Stolee > > In handle_rename_rename_1to2(), we have duplicated error handling > around colliding paths. Specifically, when we want to write out > the file and there is a directory or untracked file in the way, > we need to create a temporary file to hold the contents. This has > some special output to alert the user, and this output is > duplicated for each side of the conflict. > > Simplify the call by generating this new path in a helper > function. > > Signed-off-by: Derrick Stolee > Signed-off-by: Elijah Newren > --- > merge-recursive.c | 53 --- > 1 file changed, 27 insertions(+), 26 deletions(-) Thanks, both. Let's advance these patches to 'next' soonish.
how does "clone --filter=sparse:path" work?
[cc-ing people who worked on or seem interested in partial-clone
filters]
I've been exploring the partial-clone options a bit, and took a look at
the "sparse:path" option. I had some confusion initially, because I
expected it work something like this:
git clone --filter=sparse:path=Documentation .
But it really doesn't take an in-repo path. You have to specify a path
to a file that contains a file with .gitignore patterns. Except they're
actually _inverted_ patterns (i.e., what to include). Which confused me
again, but I guess makes sense if these are meant to be adapted from
sparse-checkout files.
So my first question is: how is this meant to be used?
I guess the idea is that somebody (the repo admin?) makes a set of
pre-made profiles, with each profile mentioning some subset of paths.
And then when you clone, you say, "I want the foo profile". How is that
profile stored and accessed?
If it's a blob in the repository, I think you can use something like
"--filter=sparse:oid=profiles:foo". And then after cloning, you'd
do "git cat-file blob profiles:foo >.git/sparse-checkout" or similar.
That seems like something that could be wrapped up in a single clone
option, but I can't find one; I won't be surprised if it simply hasn't
happened yet.
But if it's a path, then what? I'd imagine that you'd "somehow" get a
copy of the sparse profile you want out-of-band. And then you'd say "I
want to clone with the profile in this file", and then copy it into the
sparse-checkout file to do the checkout.
But the sparse-checkout file in the filter is not expanded locally, with
patterns sent over the wire. The _filename_ is sent over the wire, and
then upload-pack expands it. So you can't specify an arbitrary set of
patterns; you have to know about the profile names (how?) on the server.
That's not very flexible, though I imagine it may make certain things
easier on the server (e.g., if you pack in such a way to efficiently
serve only particular profiles).
But I'm also concerned it's dangerous. We're reading gitignore patterns
from an arbitrary name on the server's filesystem, with that name coming
from an untrusted client. So I can do:
git clone --filter=sparse:path=/etc/passwd $remote
and the server will read /etc/passwd. There's probably a lot of mischief
you can get up to with that. Sadly reading /proc/self/mem doesn't work,
because the gitignore code fstat()s the file to find out how much to
read, and the st_size there is 0. But there are probably others
(/proc/kcore is a fun one, but nobody is running their git server as
root, right?).
Below is a proof of concept script that uses this as an oracle to
explore the filesystem, as well as individual lines of files.
Should we simply be disallowing sparse:path filters over upload-pack?
-Peff
-- >8 --
# Set this to host:repo.git to see a real cross-machine connection (which makes
# it more obvious which side is reading which files). For a simulated local
# one, we'll use --no-local to make sure we really run upload-pack.
SERVER=server.git
# Do these steps manually on the remote side if you're trying it cross-server.
case "$SERVER" in
*:*)
;;
*)
# Imagine a server with a repository users can push to, with filters
enabled.
git init -q --bare $SERVER
git -C $SERVER config uploadpack.allowfilter true
# Imagine the server has a file outside of the repo we're interested in.
echo foo >/tmp/secret
;;
esac
# Some base setup.
git clone -q $SERVER local
git -C local commit -q --allow-empty -m 'some base commit'
git -C local push -q
# We can find out whether a path exists on the filesystem.
probe_file () {
# The remote upload-pack will barf if it cannot read the path $1.
rm -rf result
if git clone --bare --no-local --filter=sparse:path=$1 \
$SERVER result >/dev/null 2>&1
then
echo "$1 exists"
else
echo "$1 does not exist"
fi
}
probe_file /tmp/missing
probe_file /tmp/secret
# We can also check individual lines in a file.
probe_line () {
# Make a probe that contains the path $2.
(
cd local
echo $2 >$2
git add $2
git commit -m "probe for $2"
git push
) >/dev/null 2>&1
# And then fetch that probe with the filter to see
# if it was included. This needs to be bare so we don't
# do a followup fetch to checkout.
rm -rf result
git clone -q --bare --no-local --filter=sparse:path=$1 \
$SERVER result
# We have all the information we need now, but we have to convince Git
# to tell it to us. There's no way to set fetch_if_missing externally,
# but we can drop the remote, which means that excluded paths
# will result in an error.
git -C result remote rm origin
if git -C result cat-file -t HEAD:$2 >/dev/null 2>&1
[PATCH v3 4/4] http: change http.version value type
From: Force Charlie
Signed-off-by: Force Charlie
---
http.c | 39 ++-
1 file changed, 30 insertions(+), 9 deletions(-)
diff --git a/http.c b/http.c
index 86e454cff5..0ad797caea 100644
--- a/http.c
+++ b/http.c
@@ -48,7 +48,7 @@ char curl_errorstr[CURL_ERROR_SIZE];
static int curl_ssl_verify = -1;
static int curl_ssl_try;
-static int curl_http_version = 0;
+static const char *curl_http_version = NULL;
static const char *ssl_cert;
static const char *ssl_cipherlist;
static const char *ssl_version;
@@ -286,8 +286,7 @@ static void process_curl_messages(void)
static int http_options(const char *var, const char *value, void *cb)
{
if (!strcmp("http.version",var)) {
- curl_http_version=git_config_int(var,value);
- return 0;
+ return git_config_string(_http_version, var, value);
}
if (!strcmp("http.sslverify", var)) {
curl_ssl_verify = git_config_bool(var, value);
@@ -794,6 +793,30 @@ static long get_curl_allowed_protocols(int from_user)
}
#endif
+#if LIBCURL_VERSION_NUM >=0x072f00
+static int get_curl_http_version_opt(const char *version_string, long *opt)
+{
+ int i;
+ static struct {
+ const char *name;
+ long opt_token;
+ } choice[] = {
+ { "HTTP/1.1", CURL_HTTP_VERSION_1_1 },
+ { "HTTP/2", CURL_HTTP_VERSION_2 }
+ };
+
+ for (i = 0; i < ARRAY_SIZE(choice); i++) {
+ if (!strcmp(version_string, choice[i].name)) {
+ *opt = choice[i].opt_token;
+ return 0;
+ }
+ }
+
+ return -1; /* not found */
+}
+
+#endif
+
static CURL *get_curl_handle(void)
{
CURL *result = curl_easy_init();
@@ -812,12 +835,10 @@ static CURL *get_curl_handle(void)
}
#if LIBCURL_VERSION_NUM >= 0x072f00 // 7.47.0
-// curl_http_version 0 is default.
-if (curl_http_version == 20) {
- /* Enable HTTP2*/
- curl_easy_setopt(result,
CURLOPT_HTTP_VERSION,CURL_HTTP_VERSION_2TLS);
-} else if (curl_http_version == 11) {
- curl_easy_setopt(result,
CURLOPT_HTTP_VERSION,CURL_HTTP_VERSION_1_1);
+long opt=-1;
+if (curl_http_version &&!get_curl_http_version_opt(curl_http_version,
)) {
+ /* Set request use http version */
+ curl_easy_setopt(result, CURLOPT_HTTP_VERSION,opt);
}
#endif
--
gitgitgadget
[PATCH v3 0/4] http: add support selecting http version
Normally, git doesn't need to set curl to select the HTTP version, it works fine without HTTP/2. Adding HTTP/2 support is a icing on the cake. This patch support force enable HTTP/2 or HTTP/1.1. example: GIT_CURL_VERBOSE=1 git2 -c http.version=HTTP/2 ls-remote https://bitbucket.org/aquariusjay/deeplab-public-ver2.git Force Charlie (4): http: add support selecting http version support force use http 1.1 fix curl version to support CURL_HTTP_VERSION_2TLS http: change http.version value type http.c | 36 1 file changed, 36 insertions(+) base-commit: 8858448bb49332d353febc078ce4a3abcc962efe Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-69%2Ffcharlie%2Fmaster-v3 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-69/fcharlie/master-v3 Pull-Request: https://github.com/gitgitgadget/git/pull/69 Range-diff vs v2: 1: 4f5a935c43 = 1: 4f5a935c43 http: add support selecting http version 2: 06e9685d2b = 2: 06e9685d2b support force use http 1.1 3: eee67d8356 = 3: eee67d8356 fix curl version to support CURL_HTTP_VERSION_2TLS -: -- > 4: ef975b6093 http: change http.version value type -- gitgitgadget
[PATCH v3 1/4] http: add support selecting http version
From: Force Charlie
Signed-off-by: Force Charlie
---
http.c | 12
1 file changed, 12 insertions(+)
diff --git a/http.c b/http.c
index 3dc8c560d6..99cb04faba 100644
--- a/http.c
+++ b/http.c
@@ -48,6 +48,7 @@ char curl_errorstr[CURL_ERROR_SIZE];
static int curl_ssl_verify = -1;
static int curl_ssl_try;
+static int curl_http_version = 11;
static const char *ssl_cert;
static const char *ssl_cipherlist;
static const char *ssl_version;
@@ -284,6 +285,10 @@ static void process_curl_messages(void)
static int http_options(const char *var, const char *value, void *cb)
{
+ if (!strcmp("http.version",var)) {
+ curl_http_version=git_config_int(var,value);
+ return 0;
+ }
if (!strcmp("http.sslverify", var)) {
curl_ssl_verify = git_config_bool(var, value);
return 0;
@@ -806,6 +811,13 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_SSL_VERIFYHOST, 2);
}
+#if LIBCURL_VERSION_NUM >= 0x073100
+ if(curl_http_version == 20){
+ /* CURL Enable HTTP2*/
+ curl_easy_setopt(result,
CURLOPT_HTTP_VERSION,CURL_HTTP_VERSION_2);
+ }
+#endif
+
#if LIBCURL_VERSION_NUM >= 0x070907
curl_easy_setopt(result, CURLOPT_NETRC, CURL_NETRC_OPTIONAL);
#endif
--
gitgitgadget
[PATCH v3 2/4] support force use http 1.1
From: Force Charlie
Signed-off-by: Force Charlie
---
http.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/http.c b/http.c
index 99cb04faba..b2ec31aef5 100644
--- a/http.c
+++ b/http.c
@@ -48,7 +48,7 @@ char curl_errorstr[CURL_ERROR_SIZE];
static int curl_ssl_verify = -1;
static int curl_ssl_try;
-static int curl_http_version = 11;
+static int curl_http_version = 0;
static const char *ssl_cert;
static const char *ssl_cipherlist;
static const char *ssl_version;
@@ -811,11 +811,14 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_SSL_VERIFYHOST, 2);
}
-#if LIBCURL_VERSION_NUM >= 0x073100
- if(curl_http_version == 20){
- /* CURL Enable HTTP2*/
- curl_easy_setopt(result,
CURLOPT_HTTP_VERSION,CURL_HTTP_VERSION_2);
- }
+#if LIBCURL_VERSION_NUM >= 0x074700
+// curl_http_version 0 is default.
+if (curl_http_version == 20) {
+ /* Enable HTTP2 when request TLS*/
+ curl_easy_setopt(result,
CURLOPT_HTTP_VERSION,CURL_HTTP_VERSION_2TLS);
+} else if (curl_http_version == 11) {
+ curl_easy_setopt(result,
CURLOPT_HTTP_VERSION,CURL_HTTP_VERSION_1_1);
+}
#endif
#if LIBCURL_VERSION_NUM >= 0x070907
--
gitgitgadget
[PATCH v3 3/4] fix curl version to support CURL_HTTP_VERSION_2TLS
From: Force Charlie
Signed-off-by: Force Charlie
---
http.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/http.c b/http.c
index b2ec31aef5..86e454cff5 100644
--- a/http.c
+++ b/http.c
@@ -811,10 +811,10 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_SSL_VERIFYHOST, 2);
}
-#if LIBCURL_VERSION_NUM >= 0x074700
+#if LIBCURL_VERSION_NUM >= 0x072f00 // 7.47.0
// curl_http_version 0 is default.
if (curl_http_version == 20) {
- /* Enable HTTP2 when request TLS*/
+ /* Enable HTTP2*/
curl_easy_setopt(result,
CURLOPT_HTTP_VERSION,CURL_HTTP_VERSION_2TLS);
} else if (curl_http_version == 11) {
curl_easy_setopt(result,
CURLOPT_HTTP_VERSION,CURL_HTTP_VERSION_1_1);
--
gitgitgadget
[PATCH v5 08/10] merge-recursive: improve rename/rename(1to2)/add[/add] handling
When we have a rename/rename(1to2) conflict, each of the renames can
collide with a file addition. Each of these rename/add conflicts suffered
from the same kinds of problems that normal rename/add suffered from.
Make the code use handle_file_conflicts() as well so that we get all the
same fixes and consistent behavior between the different conflict types.
Signed-off-by: Elijah Newren
---
merge-recursive.c| 154 +--
t/t6042-merge-rename-corner-cases.sh | 29 +++--
t/t6043-merge-rename-directories.sh | 24 +++--
3 files changed, 113 insertions(+), 94 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 88e9e1166a..3e2a63d094 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1710,80 +1710,17 @@ static int handle_rename_add(struct merge_options *o,
ci->dst_entry1->stages[other_stage].mode);
}
-static int handle_file(struct merge_options *o,
- struct diff_filespec *rename,
- int stage,
- struct rename_conflict_info *ci)
-{
- char *dst_name = rename->path;
- struct stage_data *dst_entry;
- const char *cur_branch, *other_branch;
- struct diff_filespec other;
- struct diff_filespec *add;
- int ret;
-
- if (stage == 2) {
- dst_entry = ci->dst_entry1;
- cur_branch = ci->branch1;
- other_branch = ci->branch2;
- } else {
- dst_entry = ci->dst_entry2;
- cur_branch = ci->branch2;
- other_branch = ci->branch1;
- }
-
- add = filespec_from_entry(, dst_entry, stage ^ 1);
- if (add) {
- int ren_src_was_dirty = was_dirty(o, rename->path);
- char *add_name = unique_path(o, rename->path, other_branch);
- if (update_file(o, 0, >oid, add->mode, add_name))
- return -1;
-
- if (ren_src_was_dirty) {
- output(o, 1, _("Refusing to lose dirty file at %s"),
- rename->path);
- }
- /*
-* Because the double negatives somehow keep confusing me...
-*1) update_wd iff !ren_src_was_dirty.
-*2) no_wd iff !update_wd
-*3) so, no_wd == !!ren_src_was_dirty == ren_src_was_dirty
-*/
- remove_file(o, 0, rename->path, ren_src_was_dirty);
- dst_name = unique_path(o, rename->path, cur_branch);
- } else {
- if (dir_in_way(rename->path, !o->call_depth, 0)) {
- dst_name = unique_path(o, rename->path, cur_branch);
- output(o, 1, _("%s is a directory in %s adding as %s
instead"),
- rename->path, other_branch, dst_name);
- } else if (!o->call_depth &&
- would_lose_untracked(rename->path)) {
- dst_name = unique_path(o, rename->path, cur_branch);
- output(o, 1, _("Refusing to lose untracked file at %s; "
- "adding as %s instead"),
- rename->path, dst_name);
- }
- }
- if ((ret = update_file(o, 0, >oid, rename->mode, dst_name)))
- ; /* fall through, do allow dst_name to be released */
- else if (stage == 2)
- ret = update_stages(o, rename->path, NULL, rename, add);
- else
- ret = update_stages(o, rename->path, NULL, add, rename);
-
- if (dst_name != rename->path)
- free(dst_name);
-
- return ret;
-}
-
static int handle_rename_rename_1to2(struct merge_options *o,
struct rename_conflict_info *ci)
{
/* One file was renamed in both branches, but to different names. */
+ struct merge_file_info mfi;
+ struct diff_filespec other;
+ struct diff_filespec *add;
struct diff_filespec *one = ci->pair1->one;
struct diff_filespec *a = ci->pair1->two;
struct diff_filespec *b = ci->pair2->two;
+ char *path_desc;
output(o, 1, _("CONFLICT (rename/rename): "
"Rename \"%s\"->\"%s\" in branch \"%s\" "
@@ -1791,15 +1728,16 @@ static int handle_rename_rename_1to2(struct
merge_options *o,
one->path, a->path, ci->branch1,
one->path, b->path, ci->branch2,
o->call_depth ? _(" (left unresolved)") : "");
- if (o->call_depth) {
- struct merge_file_info mfi;
- struct diff_filespec other;
- struct diff_filespec *add;
- if (merge_mode_and_contents(o, one, a, b, one->path,
- ci->branch1, ci->branch2,
- o->call_depth * 2, ))
-
[PATCH v5 01/10] Add testcases for consistency in file collision conflict handling
Add testcases dealing with file collisions for the following types of
conflicts:
* add/add
* rename/add
* rename/rename(2to1)
All these conflict types simplify down to two files "colliding"
and should thus be handled similarly. This means that rename/add and
rename/rename(2to1) conflicts need to be modified to behave the same as
add/add conflicts currently do: the colliding files should be two-way
merged (instead of the current behavior of writing the two colliding
files out to separate temporary unique pathnames). Add testcases which
check this; subsequent commits will fix the conflict handling to make
these tests pass.
Signed-off-by: Elijah Newren
---
t/t6042-merge-rename-corner-cases.sh | 162 +++
1 file changed, 162 insertions(+)
diff --git a/t/t6042-merge-rename-corner-cases.sh
b/t/t6042-merge-rename-corner-cases.sh
index b97aca7fa2..b6fed2cb9a 100755
--- a/t/t6042-merge-rename-corner-cases.sh
+++ b/t/t6042-merge-rename-corner-cases.sh
@@ -937,4 +937,166 @@ test_expect_failure 'mod6-check: chains of
rename/rename(1to2) and rename/rename
)
'
+test_conflicts_with_adds_and_renames() {
+ sideL=$1
+ sideR=$2
+ expect=$3
+
+ # Setup:
+ # L
+ # / \
+ # master ?
+ # \ /
+ # R
+ #
+ # Where:
+ # Both L and R have files named 'three' which collide. Each of
+ # the colliding files could have been involved in a rename, in
+ # which case there was a file named 'one' or 'two' that was
+ # modified on the opposite side of history and renamed into the
+ # collision on this side of history.
+ #
+ # Questions:
+ # 1) The index should contain both a stage 2 and stage 3 entry
+ # for the colliding file. Does it?
+ # 2) When renames are involved, the content merges are clean, so
+ # the index should reflect the content merges, not merely the
+ # version of the colliding file from the prior commit. Does
+ # it?
+ # 3) There should be a file in the worktree named 'three'
+ # containing the two-way merged contents of the content-merged
+ # versions of 'three' from each of the two colliding
+ # files. Is it present?
+ # 4) There should not be any three~* files in the working
+ # tree
+ test_expect_success "setup simple $sideL/$sideR conflict" '
+ test_create_repo simple_${sideL}_${sideR} &&
+ (
+ cd simple_${sideL}_${sideR} &&
+
+ # Create some related files now
+ for i in $(test_seq 1 10)
+ do
+ echo Random base content line $i
+ done >file_v1 &&
+ cp file_v1 file_v2 &&
+ echo modification >>file_v2 &&
+
+ cp file_v1 file_v3 &&
+ echo more stuff >>file_v3 &&
+ cp file_v3 file_v4 &&
+ echo yet more stuff >>file_v4 &&
+
+ # Use a tag to record both these files for simple
+ # access, and clean out these untracked files
+ git tag file_v1 $(git hash-object -w file_v1) &&
+ git tag file_v2 $(git hash-object -w file_v2) &&
+ git tag file_v3 $(git hash-object -w file_v3) &&
+ git tag file_v4 $(git hash-object -w file_v4) &&
+ git clean -f &&
+
+ # Setup original commit (or merge-base), consisting of
+ # files named "one" and "two" if renames were involved.
+ touch irrelevant_file &&
+ git add irrelevant_file &&
+ if [ $sideL = "rename" ]
+ then
+ git show file_v1 >one &&
+ git add one
+ fi &&
+ if [ $sideR = "rename" ]
+ then
+ git show file_v3 >two &&
+ git add two
+ fi &&
+ test_tick && git commit -m initial &&
+
+ git branch L &&
+ git branch R &&
+
+ # Handle the left side
+ git checkout L &&
+ if [ $sideL = "rename" ]
+ then
+ git mv one three
+ else
+ git show file_v2 >three &&
+ git add three
+ fi &&
+ if [ $sideR = "rename" ]
+ then
+ git show file_v4 >two &&
+
[PATCH v5 10/10] merge-recursive: combine error handling
From: Derrick Stolee
In handle_rename_rename_1to2(), we have duplicated error handling
around colliding paths. Specifically, when we want to write out
the file and there is a directory or untracked file in the way,
we need to create a temporary file to hold the contents. This has
some special output to alert the user, and this output is
duplicated for each side of the conflict.
Simplify the call by generating this new path in a helper
function.
Signed-off-by: Derrick Stolee
Signed-off-by: Elijah Newren
---
merge-recursive.c | 53 ---
1 file changed, 27 insertions(+), 26 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 3e2a63d094..ecf8db0b71 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1710,6 +1710,27 @@ static int handle_rename_add(struct merge_options *o,
ci->dst_entry1->stages[other_stage].mode);
}
+static char *find_path_for_conflict(struct merge_options *o,
+ const char *path,
+ const char *branch1,
+ const char *branch2)
+{
+ char *new_path = NULL;
+ if (dir_in_way(path, !o->call_depth, 0)) {
+ new_path = unique_path(o, path, branch1);
+ output(o, 1, _("%s is a directory in %s adding "
+ "as %s instead"),
+ path, branch2, new_path);
+ } else if (would_lose_untracked(path)) {
+ new_path = unique_path(o, path, branch1);
+ output(o, 1, _("Refusing to lose untracked file"
+ " at %s; adding as %s instead"),
+ path, new_path);
+ }
+
+ return new_path;
+}
+
static int handle_rename_rename_1to2(struct merge_options *o,
struct rename_conflict_info *ci)
{
@@ -1784,19 +1805,9 @@ static int handle_rename_rename_1to2(struct
merge_options *o,
>oid, add->mode) < 0)
return -1;
} else {
- char *new_path = NULL;
- if (dir_in_way(a->path, !o->call_depth, 0)) {
- new_path = unique_path(o, a->path, ci->branch1);
- output(o, 1, _("%s is a directory in %s adding "
- "as %s instead"),
- a->path, ci->branch2, new_path);
- } else if (would_lose_untracked(a->path)) {
- new_path = unique_path(o, a->path, ci->branch1);
- output(o, 1, _("Refusing to lose untracked file"
- " at %s; adding as %s instead"),
- a->path, new_path);
- }
-
+ char *new_path = find_path_for_conflict(o, a->path,
+ ci->branch1,
+ ci->branch2);
if (update_file(o, 0, , mfi.mode, new_path ?
new_path : a->path))
return -1;
free(new_path);
@@ -1813,19 +1824,9 @@ static int handle_rename_rename_1to2(struct
merge_options *o,
, mfi.mode) < 0)
return -1;
} else {
- char *new_path = NULL;
- if (dir_in_way(b->path, !o->call_depth, 0)) {
- new_path = unique_path(o, b->path, ci->branch2);
- output(o, 1, _("%s is a directory in %s adding "
- "as %s instead"),
- b->path, ci->branch1, new_path);
- } else if (would_lose_untracked(b->path)) {
- new_path = unique_path(o, b->path, ci->branch2);
- output(o, 1, _("Refusing to lose untracked file"
- " at %s; adding as %s instead"),
- b->path, new_path);
- }
-
+ char *new_path = find_path_for_conflict(o, b->path,
+ ci->branch2,
+ ci->branch1);
if (update_file(o, 0, , mfi.mode, new_path ?
new_path : b->path))
return -1;
free(new_path);
--
2.19.1.858.g526e8fe740.dirty
[PATCH v5 02/10] t6036, t6042: testcases for rename collision of already conflicting files
When a single file is renamed, it can also be modified, yielding the possibility of that renamed file having content conflicts. If two different such files are renamed into the same location, then two-way merging those files may result in nested conflicts. Add a testcase that makes sure we get this case correct, and uses different lengths of conflict markers to differentiate between the different nestings. Also add another case with an extra (i.e. third) level of conflict markers due to using merge.conflictstyle=diff3 and the virtual merge base also having conflicts present. Signed-off-by: Elijah Newren --- t/t6036-recursive-corner-cases.sh| 194 +++ t/t6042-merge-rename-corner-cases.sh | 118 2 files changed, 312 insertions(+) diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh index e1cef58f2a..f229d7e47b 100755 --- a/t/t6036-recursive-corner-cases.sh +++ b/t/t6036-recursive-corner-cases.sh @@ -1402,4 +1402,198 @@ test_expect_failure 'check conflicting modes for regular file' ' ) ' +# Setup: +# L1---L2 +# / \ / \ +# masterX? +# \ / \ / +# R1---R2 +# +# Where: +# master has two files, named 'b' and 'a' +# branches L1 and R1 both modify each of the two files in conflicting ways +# +# L2 is a merge of R1 into L1; more on it later. +# R2 is a merge of L1 into R1; more on it later. +# +# X is an auto-generated merge-base used when merging L2 and R2. +# since X is a merge of L1 and R1, it has conflicting versions of each file +# +# More about L2 and R2: +# - both resolve the conflicts in 'b' and 'a' differently +# - L2 renames 'b' to 'm' +# - R2 renames 'a' to 'm' +# +# In the end, in file 'm' we have four different conflicting files (from +# two versions of 'b' and two of 'a'). In addition, if +# merge.conflictstyle is diff3, then the base version also has +# conflict markers of its own, leading to a total of three levels of +# conflict markers. This is a pretty weird corner case, but we just want +# to ensure that we handle it as well as practical. + +test_expect_success 'setup nested conflicts' ' + test_create_repo nested_conflicts && + ( + cd nested_conflicts && + + # Create some related files now + for i in $(test_seq 1 10) + do + echo Random base content line $i + done >initial && + + cp initial b_L1 && + cp initial b_R1 && + cp initial b_L2 && + cp initial b_R2 && + cp initial a_L1 && + cp initial a_R1 && + cp initial a_L2 && + cp initial a_R2 && + + test_write_lines b b_L1 >>b_L1 && + test_write_lines b b_R1 >>b_R1 && + test_write_lines b b_L2 >>b_L2 && + test_write_lines b b_R2 >>b_R2 && + test_write_lines a a_L1 >>a_L1 && + test_write_lines a a_R1 >>a_R1 && + test_write_lines a a_L2 >>a_L2 && + test_write_lines a a_R2 >>a_R2 && + + # Setup original commit (or merge-base), consisting of + # files named "b" and "a" + cp initial b && + cp initial a && + echo b >>b && + echo a >>a && + git add b a && + test_tick && git commit -m initial && + + git branch L && + git branch R && + + # Handle the left side + git checkout L && + mv -f b_L1 b && + mv -f a_L1 a && + git add b a && + test_tick && git commit -m "version L1 of files" && + git tag L1 && + + # Handle the right side + git checkout R && + mv -f b_R1 b && + mv -f a_R1 a && + git add b a && + test_tick && git commit -m "verson R1 of files" && + git tag R1 && + + # Create first merge on left side + git checkout L && + test_must_fail git merge R1 && + mv -f b_L2 b && + mv -f a_L2 a && + git add b a && + git mv b m && + test_tick && git commit -m "left merge, rename b->m" && + git tag L2 && + + # Create first merge on right side + git checkout R && + test_must_fail git merge L1 && + mv -f b_R2 b && + mv -f a_R2 a && + git add b a && + git mv a m && + test_tick && git commit -m "right merge, rename a->m" && + git tag R2 + ) +' + +test_expect_failure 'check nested conflicts' ' + ( + cd nested_conflicts && + + git clean -f && +
[PATCH v5 05/10] merge-recursive: fix rename/add conflict handling
This makes the rename/add conflict handling make use of the new
handle_file_collision() function, which fixes several bugs and improves
things for the rename/add case significantly. Previously, rename/add
would:
* Not leave any higher order stage entries in the index, making it
appear as if there were no conflict.
* Would place the rename file at the colliding path, and move the
added file elsewhere, which combined with the lack of higher order
stage entries felt really odd. It's not clear to me why the
rename should take precedence over the add; if one should be moved
out of the way, they both probably should.
* In the recursive case, it would do a two way merge of the added
file and the version of the renamed file on the renamed side,
completely excluding modifications to the renamed file on the
unrenamed side of history.
Use the new handle_file_collision() to fix all of these issues.
Signed-off-by: Elijah Newren
---
merge-recursive.c| 137 +--
t/t6036-recursive-corner-cases.sh| 24 ++---
t/t6042-merge-rename-corner-cases.sh | 4 +-
3 files changed, 101 insertions(+), 64 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 16ba425c2d..c0aa93e3df 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -186,6 +186,7 @@ static int oid_eq(const struct object_id *a, const struct
object_id *b)
enum rename_type {
RENAME_NORMAL = 0,
RENAME_VIA_DIR,
+ RENAME_ADD,
RENAME_DELETE,
RENAME_ONE_FILE_TO_ONE,
RENAME_ONE_FILE_TO_TWO,
@@ -228,6 +229,7 @@ static inline void setup_rename_conflict_info(enum
rename_type rename_type,
struct stage_data *src_entry1,
struct stage_data *src_entry2)
{
+ int ostage1 = 0, ostage2;
struct rename_conflict_info *ci;
/*
@@ -264,18 +266,22 @@ static inline void setup_rename_conflict_info(enum
rename_type rename_type,
dst_entry2->rename_conflict_info = ci;
}
- if (rename_type == RENAME_TWO_FILES_TO_ONE) {
- /*
-* For each rename, there could have been
-* modifications on the side of history where that
-* file was not renamed.
-*/
- int ostage1 = o->branch1 == branch1 ? 3 : 2;
- int ostage2 = ostage1 ^ 1;
+ /*
+* For each rename, there could have been
+* modifications on the side of history where that
+* file was not renamed.
+*/
+ if (rename_type == RENAME_ADD ||
+ rename_type == RENAME_TWO_FILES_TO_ONE) {
+ ostage1 = o->branch1 == branch1 ? 3 : 2;
ci->ren1_other.path = pair1->one->path;
oidcpy(>ren1_other.oid, _entry1->stages[ostage1].oid);
ci->ren1_other.mode = src_entry1->stages[ostage1].mode;
+ }
+
+ if (rename_type == RENAME_TWO_FILES_TO_ONE) {
+ ostage2 = ostage1 ^ 1;
ci->ren2_other.path = pair2->one->path;
oidcpy(>ren2_other.oid, _entry2->stages[ostage2].oid);
@@ -1560,7 +1566,6 @@ static struct diff_filespec *filespec_from_entry(struct
diff_filespec *target,
return target;
}
-#if 0 // #if-0-ing avoids unused function warning; will make live in next
commit
static int handle_file_collision(struct merge_options *o,
const char *collide_path,
const char *prev_path1,
@@ -1576,6 +1581,20 @@ static int handle_file_collision(struct merge_options *o,
char *alt_path = NULL;
const char *update_path = collide_path;
+ /*
+* It's easiest to get the correct things into stage 2 and 3, and
+* to make sure that the content merge puts HEAD before the other
+* branch if we just ensure that branch1 == o->branch1. So, simply
+* flip arguments around if we don't have that.
+*/
+ if (branch1 != o->branch1) {
+ return handle_file_collision(o, collide_path,
+prev_path2, prev_path1,
+branch2, branch1,
+b_oid, b_mode,
+a_oid, a_mode);
+ }
+
/*
* In the recursive case, we just opt to undo renames
*/
@@ -1679,7 +1698,38 @@ static int handle_file_collision(struct merge_options *o,
*/
return mfi.clean;
}
-#endif
+
+static int handle_rename_add(struct merge_options *o,
+struct rename_conflict_info *ci)
+{
+ /* a was renamed to c, and a separate c was added. */
+ struct diff_filespec *a = ci->pair1->one;
+ struct diff_filespec *c = ci->pair1->two;
+ char *path = c->path;
+ char
[PATCH v5 03/10] merge-recursive: increase marker length with depth of recursion
Later patches in this series will modify file collision conflict
handling (e.g. from rename/add and rename/rename(2to1) conflicts) so
that multiply nested conflict markers can arise even before considering
conflicts in the virtual merge base. Including the virtual merge base
will provide a way to get triply (or higher) nested conflict markers.
This new way to get nested conflict markers will force the need for a
more general mechanism to extend the length of conflict markers in order
to differentiate between different nestings.
Along with this change to conflict marker length handling, we want to
make sure that we don't regress handling for other types of conflicts
with nested conflict markers. Add a more involved testcase using
merge.conflictstyle=diff3, where not only does the virtual merge base
contain conflicts, but its virtual merge base does as well (i.e. a case
with triply nested conflict markers). While there are multiple
reasonable ways to handle nested conflict markers in the virtual merge
base for this type of situation, the easiest approach that dovetails
well with the new needs for the file collision conflict handling is to
require that the length of the conflict markers increase with each
subsequent nesting.
Subsequent patches which change the rename/add and rename/rename(2to1)
conflict handling will modify the extra_marker_size flag appropriately
for their new needs.
Signed-off-by: Elijah Newren
---
ll-merge.c| 4 +-
ll-merge.h| 1 +
merge-recursive.c | 25 +++--
t/t6036-recursive-corner-cases.sh | 151 ++
4 files changed, 172 insertions(+), 9 deletions(-)
diff --git a/ll-merge.c b/ll-merge.c
index 3c8fb917e9..5b8d46aede 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -384,7 +384,9 @@ int ll_merge(mmbuffer_t *result_buf,
if (opts->virtual_ancestor) {
if (driver->recursive)
driver = find_ll_merge_driver(driver->recursive);
- marker_size += 2;
+ }
+ if (opts->extra_marker_size) {
+ marker_size += opts->extra_marker_size;
}
return driver->fn(driver, result_buf, path, ancestor, ancestor_label,
ours, our_label, theirs, their_label,
diff --git a/ll-merge.h b/ll-merge.h
index 6c6e22e40d..b9e2af1c88 100644
--- a/ll-merge.h
+++ b/ll-merge.h
@@ -13,6 +13,7 @@ struct ll_merge_options {
unsigned virtual_ancestor : 1;
unsigned variant : 2; /* favor ours, favor theirs, or union merge */
unsigned renormalize : 1;
+ unsigned extra_marker_size;
long xdl_opts;
};
diff --git a/merge-recursive.c b/merge-recursive.c
index acc2f64a4e..f35e3b5f95 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1058,7 +1058,8 @@ static int merge_3way(struct merge_options *o,
const struct diff_filespec *a,
const struct diff_filespec *b,
const char *branch1,
- const char *branch2)
+ const char *branch2,
+ const int extra_marker_size)
{
mmfile_t orig, src1, src2;
struct ll_merge_options ll_opts = {0};
@@ -1066,6 +1067,7 @@ static int merge_3way(struct merge_options *o,
int merge_status;
ll_opts.renormalize = o->renormalize;
+ ll_opts.extra_marker_size = extra_marker_size;
ll_opts.xdl_opts = o->xdl_opts;
if (o->call_depth) {
@@ -1301,6 +1303,7 @@ static int merge_mode_and_contents(struct merge_options
*o,
const char *filename,
const char *branch1,
const char *branch2,
+ const int extra_marker_size,
struct merge_file_info *result)
{
if (o->branch1 != branch1) {
@@ -1311,7 +1314,8 @@ static int merge_mode_and_contents(struct merge_options
*o,
*/
return merge_mode_and_contents(o, one, b, a,
filename,
- branch2, branch1, result);
+ branch2, branch1,
+ extra_marker_size, result);
}
result->merge = 0;
@@ -1352,7 +1356,8 @@ static int merge_mode_and_contents(struct merge_options
*o,
int ret = 0, merge_status;
merge_status = merge_3way(o, _buf, one, a, b,
- branch1, branch2);
+ branch1, branch2,
+ extra_marker_size);
if ((merge_status < 0) || !result_buf.ptr)
ret = err(o, _("Failed to execute internal
merge"));
@@ -1641,7
[PATCH v5 04/10] merge-recursive: new function for better colliding conflict resolutions
There are three conflict types that represent two (possibly entirely unrelated) files colliding at the same location: * add/add * rename/add * rename/rename(2to1) These three conflict types already share more similarity than might be immediately apparent from their description: (1) the handling of the rename variants already involves removing any entries from the index corresponding to the original file names[*], thus only leaving entries in the index for the colliding path; (2) likewise, any trace of the original file name in the working tree is also removed. So, in all three cases we're left with how to represent two colliding files in both the index and the working copy. [*] Technically, this isn't quite true because rename/rename(2to1) conflicts in the recursive (o->call_depth > 0) case do an "unrename" since about seven years ago. But even in that case, Junio felt compelled to explain that my decision to "unrename" wasn't necessarily the only or right answer -- search for "Comment from Junio" in t6036 for details. My initial motivation for looking at these three conflict types was that if the handling of these three conflict types is the same, at least in the limited set of cases where a renamed file is unmodified on the side of history where the file is not renamed, then a significant performance improvement for rename detection during merges is possible. However, while that served as motivation to look at these three types of conflicts, the actual goal of this new function is to try to improve the handling for all three cases, not to merely make them the same as each other in that special circumstance. === Handling the working tree === The previous behavior for these conflict types in regards to the working tree (assuming the file collision occurs at 'foo') was: * add/add does a two-way merge of the two files and records it as 'foo'. * rename/rename(2to1) records the two different files into two new uniquely named files (foo~HEAD and foo~$MERGE), while removing 'foo' from the working tree. * rename/add records the two different files into two different locations, recording the add at foo~$SIDE and, oddly, recording the rename at foo (why is the rename more important than the add?) So, the question for what to write to the working tree boils down to whether the two colliding files should be two-way merged and recorded in place, or recorded into separate files. As per discussion on the git mailing lit, two-way merging was deemed to always be preferred, as that makes these cases all more like content conflicts that users can handle from within their favorite editor, IDE, or merge tool. Note that since renames already involve a content merge, rename/add and rename/rename(2to1) conflicts could result in nested conflict markers. === Handling of the index === For a typical rename, unpack_trees() would set up the index in the following fashion: old_path new_path stage1: 5ca1ab1e stage2: f005ba11 stage3: b0a710ad And merge-recursive would rewrite this to new_path stage1: 5ca1ab1e stage2: f005ba11 stage3: b0a710ad Removing old_path from the index means the user won't have to `git rm old_path` manually every time a renamed path has a content conflict. It also means they can use `git checkout [--ours|--theirs|--conflict|-m] new_path`, `git diff [--ours|--theirs]` and various other commands that would be difficult otherwise. This strategy becomes a problem when we have a rename/add or rename/rename(2to1) conflict, however, because then we have only three slots to store blob sha1s and we need either four or six. Previously, this was handled by continuing to delete old_path from the index, and just outright ignoring any blob shas from old_path. That had the downside of deleting any trace of changes made to old_path on the other side of history. This function instead does a three-way content merge of the renamed file, and stores the blob sha1 for that at either stage2 or stage3 for new_path (depending on which side the rename came from). That has the advantage of bringing information about changes on both sides and still allows for easy resolution (no need to git rm old_path, etc.), but does have the downside that if the content merge had conflict markers, then what we store in the index is the sha1 of a blob with conflict markers. While that is a downside, it seems less problematic than the downsides of any obvious alternatives, and certainly makes more sense than the previous handling. Further, it has a precedent in that when we do recursive merges, we may accept a file with conflict markers as the resolution for the merge of the merge-bases, which will then show up in the index of the outer merge at stage 1 if a conflict exists at the outer level. Signed-off-by: Elijah Newren --- merge-recursive.c | 121 ++ 1 file changed, 121 insertions(+) diff
[PATCH v5 06/10] merge-recursive: improve handling for rename/rename(2to1) conflicts
This makes the rename/rename(2to1) conflicts use the new
handle_file_collision() function. Since that function was based
originally on the rename/rename(2to1) handling code, the main
differences here are in what was added. In particular:
* Instead of storing files at collide_path~HEAD and collide_path~MERGE,
the files are two-way merged and recorded at collide_path.
* Instead of recording the version of the renamed file that existed
on the renamed side in the index (thus ignoring any changes that
were made to the file on the side of history without the rename),
we do a three-way content merge on the renamed path, then store
that at either stage 2 or stage 3.
* Note that since the content merge for each rename may have conflicts,
and then we have to merge the two renamed files, we can end up with
nested conflict markers.
Signed-off-by: Elijah Newren
---
merge-recursive.c| 104 ---
t/t6036-recursive-corner-cases.sh| 12 +---
t/t6042-merge-rename-corner-cases.sh | 38 ++
t/t6043-merge-rename-directories.sh | 83 -
4 files changed, 89 insertions(+), 148 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index c0aa93e3df..62eab9e4cc 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -696,27 +696,6 @@ static int update_stages(struct merge_options *opt, const
char *path,
return 0;
}
-static int update_stages_for_stage_data(struct merge_options *opt,
- const char *path,
- const struct stage_data *stage_data)
-{
- struct diff_filespec o, a, b;
-
- o.mode = stage_data->stages[1].mode;
- oidcpy(, _data->stages[1].oid);
-
- a.mode = stage_data->stages[2].mode;
- oidcpy(, _data->stages[2].oid);
-
- b.mode = stage_data->stages[3].mode;
- oidcpy(, _data->stages[3].oid);
-
- return update_stages(opt, path,
-is_null_oid() ? NULL : ,
-is_null_oid() ? NULL : ,
-is_null_oid() ? NULL : );
-}
-
static void update_entry(struct stage_data *entry,
struct diff_filespec *o,
struct diff_filespec *a,
@@ -1871,7 +1850,6 @@ static int handle_rename_rename_2to1(struct merge_options
*o,
char *path_side_2_desc;
struct merge_file_info mfi_c1;
struct merge_file_info mfi_c2;
- int ret;
output(o, 1, _("CONFLICT (rename/rename): "
"Rename %s->%s in %s. "
@@ -1879,81 +1857,22 @@ static int handle_rename_rename_2to1(struct
merge_options *o,
a->path, c1->path, ci->branch1,
b->path, c2->path, ci->branch2);
- remove_file(o, 1, a->path, o->call_depth ||
would_lose_untracked(a->path));
- remove_file(o, 1, b->path, o->call_depth ||
would_lose_untracked(b->path));
-
path_side_1_desc = xstrfmt("version of %s from %s", path, a->path);
path_side_2_desc = xstrfmt("version of %s from %s", path, b->path);
if (merge_mode_and_contents(o, a, c1, >ren1_other, path_side_1_desc,
o->branch1, o->branch2,
- o->call_depth * 2, _c1) ||
+ 1 + o->call_depth * 2, _c1) ||
merge_mode_and_contents(o, b, >ren2_other, c2, path_side_2_desc,
o->branch1, o->branch2,
- o->call_depth * 2, _c2))
+ 1 + o->call_depth * 2, _c2))
return -1;
free(path_side_1_desc);
free(path_side_2_desc);
- if (o->call_depth) {
- /*
-* If mfi_c1.clean && mfi_c2.clean, then it might make
-* sense to do a two-way merge of those results. But, I
-* think in all cases, it makes sense to have the virtual
-* merge base just undo the renames; they can be detected
-* again later for the non-recursive merge.
-*/
- remove_file(o, 0, path, 0);
- ret = update_file(o, 0, _c1.oid, mfi_c1.mode, a->path);
- if (!ret)
- ret = update_file(o, 0, _c2.oid, mfi_c2.mode,
- b->path);
- } else {
- char *new_path1 = unique_path(o, path, ci->branch1);
- char *new_path2 = unique_path(o, path, ci->branch2);
- output(o, 1, _("Renaming %s to %s and %s to %s instead"),
- a->path, new_path1, b->path, new_path2);
- if (was_dirty(o, path))
- output(o, 1, _("Refusing to lose dirty file at %s"),
- path);
- else if (would_lose_untracked(path))
- /*
-* Only
[PATCH v5 09/10] t6036, t6043: increase code coverage for file collision handling
Stolee's coverage reports found a few code blocks for file collision
conflicts that had not previously been covered by testcases; add a few
more testcases to cover those too.
Signed-off-by: Elijah Newren
---
t/t6036-recursive-corner-cases.sh | 51 +
t/t6043-merge-rename-directories.sh | 37 +
2 files changed, 88 insertions(+)
diff --git a/t/t6036-recursive-corner-cases.sh
b/t/t6036-recursive-corner-cases.sh
index 99cddb05af..b7488b00c0 100755
--- a/t/t6036-recursive-corner-cases.sh
+++ b/t/t6036-recursive-corner-cases.sh
@@ -240,6 +240,57 @@ test_expect_success 'git detects differently handled
merges conflict' '
)
'
+# Repeat the above testcase with precisely the same setup, other than with
+# the two merge bases having different orderings of commit timestamps so
+# that they are reversed in the order they are provided to merge-recursive,
+# so that we can improve code coverage.
+test_expect_success 'git detects differently handled merges conflict, swapped'
'
+ (
+ cd rename-add &&
+
+ # Difference #1: Do cleanup from previous testrun
+ git reset --hard &&
+ git clean -fdqx &&
+
+ # Difference #2: Change commit timestamps
+ btime=$(git log --no-walk --date=raw --format=%cd B | awk
"{print \$1}") &&
+ ctime=$(git log --no-walk --date=raw --format=%cd C | awk
"{print \$1}") &&
+ newctime=$(($btime+1)) &&
+ git fast-export --no-data --all | sed -e s/$ctime/$newctime/ |
git fast-import --force --quiet &&
+ # End of differences; rest is copy-paste of last test
+
+ git checkout D^0 &&
+ test_must_fail git merge -s recursive E^0 &&
+
+ git ls-files -s >out &&
+ test_line_count = 3 out &&
+ git ls-files -u >out &&
+ test_line_count = 3 out &&
+ git ls-files -o >out &&
+ test_line_count = 1 out &&
+
+ git rev-parse >expect \
+ C:new_a D:new_a E:new_a &&
+ git rev-parse >actual \
+ :1:new_a :2:new_a :3:new_a &&
+ test_cmp expect actual &&
+
+ # Test that the two-way merge in new_a is as expected
+ git cat-file -p D:new_a >ours &&
+ git cat-file -p E:new_a >theirs &&
+ >empty &&
+ test_must_fail git merge-file \
+ -L "HEAD" \
+ -L "" \
+ -L "E^0" \
+ ours empty theirs &&
+ sed -e "s/^\([<=>]\)/\1\1\1/" ours >expect &&
+ git hash-object new_a >actual &&
+ git hash-object ours >expect &&
+ test_cmp expect actual
+ )
+'
+
#
# criss-cross + modify/delete:
#
diff --git a/t/t6043-merge-rename-directories.sh
b/t/t6043-merge-rename-directories.sh
index 5c01a0c14a..62c564707b 100755
--- a/t/t6043-merge-rename-directories.sh
+++ b/t/t6043-merge-rename-directories.sh
@@ -3163,6 +3163,43 @@ test_expect_success '10c-check: Overwrite untracked with
dir rename/rename(1to2)
)
'
+test_expect_success '10c-check: Overwrite untracked with dir
rename/rename(1to2), other direction' '
+ (
+ cd 10c &&
+
+ git reset --hard &&
+ git clean -fdqx &&
+
+ git checkout B^0 &&
+ mkdir y &&
+ echo important >y/c &&
+
+ test_must_fail git merge -s recursive A^0 >out 2>err &&
+ test_i18ngrep "CONFLICT (rename/rename)" out &&
+ test_i18ngrep "Refusing to lose untracked file at y/c; adding
as y/c~HEAD instead" out &&
+
+ git ls-files -s >out &&
+ test_line_count = 6 out &&
+ git ls-files -u >out &&
+ test_line_count = 3 out &&
+ git ls-files -o >out &&
+ test_line_count = 3 out &&
+
+ git rev-parse >actual \
+ :0:y/a :0:y/b :0:x/d :1:x/c :3:w/c :2:y/c &&
+ git rev-parse >expect \
+O:z/a O:z/b O:x/d O:x/c O:x/c O:x/c &&
+ test_cmp expect actual &&
+
+ git hash-object y/c~HEAD >actual &&
+ git rev-parse O:x/c >expect &&
+ test_cmp expect actual &&
+
+ echo important >expect &&
+ test_cmp expect y/c
+ )
+'
+
# Testcase 10d, Delete untracked w/ dir rename/rename(2to1)
# Commit O: z/{a,b,c_1},x/{d,e,f_2}
# Commit A: y/{a,b},x/{d,e,f_2,wham_1} + untracked y/wham
--
2.19.1.858.g526e8fe740.dirty
[PATCH v5 07/10] merge-recursive: use handle_file_collision for add/add conflicts
This results in no-net change of behavior, it simply ensures that all
file-collision conflict handling types are being handled the same by
calling the same function.
Signed-off-by: Elijah Newren
---
merge-recursive.c | 29 +
1 file changed, 21 insertions(+), 8 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 62eab9e4cc..88e9e1166a 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -3356,14 +3356,27 @@ static int process_entry(struct merge_options *o,
clean_merge = -1;
}
} else if (a_oid && b_oid) {
- /* Case C: Added in both (check for same permissions) and */
- /* case D: Modified in both, but differently. */
- int is_dirty = 0; /* unpack_trees would have bailed if dirty */
- clean_merge = handle_content_merge(o, path, is_dirty,
- o_oid, o_mode,
- a_oid, a_mode,
- b_oid, b_mode,
- NULL);
+ if (!o_oid) {
+ /* Case C: Added in both (check for same permissions) */
+ output(o, 1,
+ _("CONFLICT (add/add): Merge conflict in %s"),
+ path);
+ clean_merge = handle_file_collision(o,
+ path, NULL, NULL,
+ o->branch1,
+ o->branch2,
+ a_oid, a_mode,
+ b_oid, b_mode);
+ } else {
+ /* case D: Modified in both, but differently. */
+ int is_dirty = 0; /* unpack_trees would have bailed if
dirty */
+ clean_merge = handle_content_merge(o, path,
+ is_dirty,
+ o_oid, o_mode,
+ a_oid, a_mode,
+ b_oid, b_mode,
+ NULL);
+ }
} else if (!o_oid && !a_oid && !b_oid) {
/*
* this entry was deleted altogether. a_mode == 0 means
--
2.19.1.858.g526e8fe740.dirty
[PATCH v5 00/10] Improve path collision conflict resolutions
This series applies cleanly on master, since en/merge-cleanup-more has now merged down. This series makes all the "file collision" conflict types be handled consistently; making them all behave like add/add (as suggested by Jonathan[1] and Junio[2]). These types are: * add/add * rename/add * rename/rename(2to1) * each rename/add piece of a rename/rename(1to2)/add[/add] conflict [1] https://public-inbox.org/git/20180312213521.gb58...@aiede.svl.corp.google.com/ [2] https://public-inbox.org/git/capc5davu8vv9rdgon8jixeo3ycdvqq38yszzc-cpo+aqcak...@mail.gmail.com Changes since v4: * Merged the two RFC patches into a single patch. * Added patch submitted by Stolee at the end. * (The addition of what are the last two patches are also the only difference since v3) Derrick Stolee (1): merge-recursive: combine error handling Elijah Newren (9): Add testcases for consistency in file collision conflict handling t6036, t6042: testcases for rename collision of already conflicting files merge-recursive: increase marker length with depth of recursion merge-recursive: new function for better colliding conflict resolutions merge-recursive: fix rename/add conflict handling merge-recursive: improve handling for rename/rename(2to1) conflicts merge-recursive: use handle_file_collision for add/add conflicts merge-recursive: improve rename/rename(1to2)/add[/add] handling t6036, t6043: increase code coverage for file collision handling ll-merge.c | 4 +- ll-merge.h | 1 + merge-recursive.c| 529 --- t/t6036-recursive-corner-cases.sh| 430 +- t/t6042-merge-rename-corner-cases.sh | 333 - t/t6043-merge-rename-directories.sh | 144 +--- 6 files changed, 1149 insertions(+), 292 deletions(-) -- 2.19.1.858.g526e8fe740.dirty
Re: [PATCH] branch: make --show-current use already resolved HEAD
I did something that resulted in the mailing list not being cc'd. Apologies to Junio and Daniels for the double send. :( On Thu, Nov 08, 2018 at 10:11:02AM +0900, Junio C Hamano wrote: > I'd prefer to see scriptors avoid using "git branch", too. > > Unlike end-user facing documentation where we promise "we do X and > will continue to do so because of Y" to the readers, the log message > is primarily for recording the original motivation of the change, so > that we can later learn "we did X back then because we thought Y". > When we want to revise X, we revisit if the reason Y is still valid. > > So in that sense, the door to "break" the scriptability is still > open. > Over at #git, commit messages are sometimes consulted to disambiguate or clarify certain details. Often the documentation is correct but people dispute over interpretations. If someone came asking if `git branch` is parsable, I would advise against and direct them to the plumbing or format alternative. But if someone came over with a link to this commit asking the same question, I suspect the answer would be: it's probably safe to parse the output of this specific option because the commit says so. Thanks for clarifying this is wrong. > > > > static const char *head; > > static struct object_id head_oid; > > +static int head_flags = 0; > > You've eliminated the "now unnecessary" helper and do everything > inside cmd_branch(), so perhaps this can be made function local, no? > I was not sure if these 3 lines were global intentionally or if it was just an artifact from the past. Since it looks like the latter, I'll make them local. -- Rafael Ascensão
Re: [PATCH 0/1] http: add support selecting http version
"brian m. carlson" writes: > On Wed, Nov 07, 2018 at 02:44:51PM +0100, Daniel Stenberg wrote: >> On Wed, 7 Nov 2018, Force.Charlie-I via GitGitGadget wrote: >> >> > Normally, git doesn't need to set curl to select the HTTP version, it >> > works fine without HTTP2. Adding HTTP2 support is a icing on the cake. >> >> Just a FYI: >> >> Starting with libcurl 7.62.0 (released a week ago), it now defaults to the >> "2TLS" setting unless you tell it otherwise. With 2TLS, libcurl will attempt >> to use HTTP/2 for HTTPS URLs. > > With this information, I think I would rather we rely on libcurl to do > this rather than putting it in Git. Users will automatically get the > best supported protocol instead of having to configure it manually. Yup. I suspect that the mechanism _might_ turn out to be useful to force downgrading, though.
Re: [PATCH/RFC] Support --append-trailer in cherry-pick and revert
Jeff King writes: > So if we are comfortable with saying that this is a new feature to have > the machine-readable trailer version, and there isn't a robust way to > get historical revert information (because there really isn't[1]), then > I think we can just punt on any kind of trailer-normalization magic. Yes, I do consider that the original suggestion was two-part - cherry-pick did have machine readable info, but by historical accident, it is shaped differently from "trailers", so we'd transition into the new format. - revert did not have machine readble info at all, so we are adding one, even though it is not that interesting as cherry-pick (for the reasons you stated in an earlier message in this thread). So my "honest answer" is your #1, "sorry, there was no machine-readable version back then", for reverts. We do not have such a problem with cherry-pick luckily ;-) > [1] Thinking back on reverts I have done, they are often _not_ > straight-up reverts. For example, I may end up dropping half of a > commit, but leaving some traces of it in place in order to build up > the correct solution on top (i.e., fixing whatever problem caused me > to revert in the first place). I list those as "this is morally a > revert of 1234abcd...", which is definitely not machine readable. ;) Yup, and it is debatable if it even makes sense to add the machine readable trailer for such a commit. A human-made claim that it is a "moral equivalent of reverting X" may not look any different from a "textual revert of X" to a machine, but the actual patch text would be quite different---unless the machine reading it understands "moral equivalence", letting it blindly take on faith whatever humans say may not be a good idea.
Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()
Jeff King writes: > I think we would want to carefully think about the call in enter_repo(). > We do not want git-daemon to accidentally expose repositories in > $RUNTIME_PREFIX. > > Looking over the code, I think this is OK. The expansion happens in > enter_repo(), and then we take the path that was found and do our > ok_paths checks on it (which makes sense -- even now you'd ask to export > "/home/" and it would need to look at "~peff/repo.git" and expand that > to "/home/peff/repo.git" before doing a simple string check. Yup, that is another reason why I think this new "expansion feature" belongs to the function, not to a wrapper that is aware of this new feature in addition to ~tilde expansion. >> Between ~ and $VARIABLE_LOOKING_THINGS, I do not have >> a strong preference either way, but I am getting an impression that >> the latter is more generally favoured in the discussion? > > I certainly prefer the latter, but I thought I was the only one to have > expressed support so far. ;) The first mention of pseudo-variable I saw was in Duy's message, wondering if $ROOT is more appropriate than "/", and I counted it as supporting the $VARIABLE syntax.
Re: [PATCH v3] i18n: make GETTEXT_POISON a runtime option
Ævar Arnfjörð Bjarmason writes: > * We error out in the Makefile if you're still saying >GETTEXT_POISON=YesPlease. I expected this would be irritating, but it turns out it is worse than mere irritation but is a severe hinderance to affect my performance, as I (and my bots) keep building and testing different versions of Git under different configurations. I know it was done to help those who only ever build a single track at a time and mostly moving forward, but I'm very much tempted to remove this part, perhaps demote it to a warning of some sort. > ifdef GETTEXT_POISON > - BASIC_CFLAGS += -DGETTEXT_POISON > +$(error The GETTEXT_POISON option has been removed in favor of runtime > GIT_TEST_GETTEXT_POISON. See t/README!) > endif -- >8 -- Makefile: ease dynamic-gettext-poison transition Earlier we made the entire build to fail when GETTEXT_POISON=Yes is given to make, to notify those who did not notice that text poisoning is now a runtime behaviour. It turns out that this too irritating for those who need to build and test different versions of Git that cross the boundary between history with and without this topic to switch between two environment variables. Demote the error to a warning, so that you can say something like make GETTEXT_POISON=Yes GIT_TEST_GETTEXT_POISON test during the transition period, without having to worry about whether exact version you are testing has or does not have this topic. Signed-off-by: Junio C Hamano --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index f3a9995e50..6b492f44a6 100644 --- a/Makefile +++ b/Makefile @@ -1447,7 +1447,7 @@ ifdef NO_SYMLINK_HEAD BASIC_CFLAGS += -DNO_SYMLINK_HEAD endif ifdef GETTEXT_POISON -$(error The GETTEXT_POISON option has been removed in favor of runtime GIT_TEST_GETTEXT_POISON. See t/README!) +$(warning The GETTEXT_POISON option has been removed in favor of runtime GIT_TEST_GETTEXT_POISON. See t/README!) endif ifdef NO_GETTEXT BASIC_CFLAGS += -DNO_GETTEXT
Re: [PATCH 1/1] http: add support selecting http version
"Force Charlie via GitGitGadget" writes:
> From: Force Charlie
>
> Signed-off-by: Force Charlie
> ---
> http.c | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/http.c b/http.c
> index 3dc8c560d6..99cb04faba 100644
> --- a/http.c
> +++ b/http.c
> @@ -48,6 +48,7 @@ char curl_errorstr[CURL_ERROR_SIZE];
>
> static int curl_ssl_verify = -1;
> static int curl_ssl_try;
> +static int curl_http_version = 11;
Is there any reason that we need to have this variable's value to be
"int"? I _think_ in this patch, the variable is used to choose
between the default and "HTTP/2", and I do not think the updated
code can choose any other new value that may be supported by an even
newer cURL library without further update, i.e. we'd need a variant of
"if the configuration asks HTTP/2 then use CURLOPT_HTTP_VERSION with
CURL_HTTP_VERSION_2" for the new choice.
So I'd think it would not add much value to force end users use a
rather cryptic "20" (vs "11") to choose between "2" and "1.1". Why
not use spell it out, e.g. using the official name of the protocol
"HTTP/2" (vs "HTTP/1.1"), with a "const char *" instead?
The new configuration variable and the possible values it can take
must be documented, of course. I think it would make the description
far less embarrassing if we say "HTTP/2" etc. rather than "20",
"11", etc.
> @@ -284,6 +285,10 @@ static void process_curl_messages(void)
>
> static int http_options(const char *var, const char *value, void *cb)
> {
> + if (!strcmp("http.version",var)) {
> + curl_http_version=git_config_int(var,value);
STYLE. Missing SP after comma, and around assignment.
> + return 0;
> + }
> if (!strcmp("http.sslverify", var)) {
> curl_ssl_verify = git_config_bool(var, value);
> return 0;
> @@ -806,6 +811,13 @@ static CURL *get_curl_handle(void)
> curl_easy_setopt(result, CURLOPT_SSL_VERIFYHOST, 2);
> }
>
> +#if LIBCURL_VERSION_NUM >= 0x073100
> + if(curl_http_version == 20){
STYLE. Missing SP before opening paren and after closing paren.
> + /* CURL Enable HTTP2*/
STYLE. Missing SP before closing asterisk-slash.
> + curl_easy_setopt(result,
> CURLOPT_HTTP_VERSION,CURL_HTTP_VERSION_2);
> + }
> +#endif
Shouldn't this block also handle the other values, e.g. "11"?
I _think_ the curl_http_version variable (be it an deci-int, or a
const char *) should be initialized to a value that you can use to
notice that the configuration did not specify any, and then this
part should become more like
if (curl_http_version &&
!get_curl_http_version_opt(curl_http_version, ))
curl_easy_setopt(result, CURL_HTTP_VERSION, opt);
with a helper function like this:
static int get_curl_http_version_opt(const char *version_string, long *opt)
{
int i;
static struct {
const char *name;
lnog opt_token;
} choice[] = {
{ "HTTP/1.1", CURL_HTTP_VERSION_1_1 },
{ "HTTP/2", CURL_HTTP_VERSION_2 },
};
for (i = 0; i < ARRAY_SIZE(choice); i++) {
if (!strcmp(version_string, choice[i].name)) {
*opt = choice[i].opt_token;
return 0;
}
}
return -1; /* not found */
}
which would make it trivial to support new values later.
> #if LIBCURL_VERSION_NUM >= 0x070907
> curl_easy_setopt(result, CURLOPT_NETRC, CURL_NETRC_OPTIONAL);
> #endif
[PATCH v2 3/3] fix curl version to support CURL_HTTP_VERSION_2TLS
From: Force Charlie
Signed-off-by: Force Charlie
---
http.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/http.c b/http.c
index b2ec31aef5..86e454cff5 100644
--- a/http.c
+++ b/http.c
@@ -811,10 +811,10 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_SSL_VERIFYHOST, 2);
}
-#if LIBCURL_VERSION_NUM >= 0x074700
+#if LIBCURL_VERSION_NUM >= 0x072f00 // 7.47.0
// curl_http_version 0 is default.
if (curl_http_version == 20) {
- /* Enable HTTP2 when request TLS*/
+ /* Enable HTTP2*/
curl_easy_setopt(result,
CURLOPT_HTTP_VERSION,CURL_HTTP_VERSION_2TLS);
} else if (curl_http_version == 11) {
curl_easy_setopt(result,
CURLOPT_HTTP_VERSION,CURL_HTTP_VERSION_1_1);
--
gitgitgadget
[PATCH v2 0/3] http: add support selecting http version
Normally, git doesn't need to set curl to select the HTTP version, it works fine without HTTP2. Adding HTTP2 support is a icing on the cake. When http.version=20 is set, git will attempt to request the server using HTTP2. If the remote server does not support HTTP2, it is no different. Currently bitbucket supports HTTP2 and is available for testing. example: GIT_CURL_VERBOSE=1 git2 -c http.version=20 ls-remote https://bitbucket.org/aquariusjay/deeplab-public-ver2.git Force Charlie (3): http: add support selecting http version support force use http 1.1 fix curl version to support CURL_HTTP_VERSION_2TLS http.c | 15 +++ 1 file changed, 15 insertions(+) base-commit: 8858448bb49332d353febc078ce4a3abcc962efe Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-69%2Ffcharlie%2Fmaster-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-69/fcharlie/master-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/69 Range-diff vs v1: 1: 4f5a935c43 = 1: 4f5a935c43 http: add support selecting http version -: -- > 2: 06e9685d2b support force use http 1.1 -: -- > 3: eee67d8356 fix curl version to support CURL_HTTP_VERSION_2TLS -- gitgitgadget
[PATCH v2 1/3] http: add support selecting http version
From: Force Charlie
Signed-off-by: Force Charlie
---
http.c | 12
1 file changed, 12 insertions(+)
diff --git a/http.c b/http.c
index 3dc8c560d6..99cb04faba 100644
--- a/http.c
+++ b/http.c
@@ -48,6 +48,7 @@ char curl_errorstr[CURL_ERROR_SIZE];
static int curl_ssl_verify = -1;
static int curl_ssl_try;
+static int curl_http_version = 11;
static const char *ssl_cert;
static const char *ssl_cipherlist;
static const char *ssl_version;
@@ -284,6 +285,10 @@ static void process_curl_messages(void)
static int http_options(const char *var, const char *value, void *cb)
{
+ if (!strcmp("http.version",var)) {
+ curl_http_version=git_config_int(var,value);
+ return 0;
+ }
if (!strcmp("http.sslverify", var)) {
curl_ssl_verify = git_config_bool(var, value);
return 0;
@@ -806,6 +811,13 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_SSL_VERIFYHOST, 2);
}
+#if LIBCURL_VERSION_NUM >= 0x073100
+ if(curl_http_version == 20){
+ /* CURL Enable HTTP2*/
+ curl_easy_setopt(result,
CURLOPT_HTTP_VERSION,CURL_HTTP_VERSION_2);
+ }
+#endif
+
#if LIBCURL_VERSION_NUM >= 0x070907
curl_easy_setopt(result, CURLOPT_NETRC, CURL_NETRC_OPTIONAL);
#endif
--
gitgitgadget
[PATCH v2 2/3] support force use http 1.1
From: Force Charlie
Signed-off-by: Force Charlie
---
http.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/http.c b/http.c
index 99cb04faba..b2ec31aef5 100644
--- a/http.c
+++ b/http.c
@@ -48,7 +48,7 @@ char curl_errorstr[CURL_ERROR_SIZE];
static int curl_ssl_verify = -1;
static int curl_ssl_try;
-static int curl_http_version = 11;
+static int curl_http_version = 0;
static const char *ssl_cert;
static const char *ssl_cipherlist;
static const char *ssl_version;
@@ -811,11 +811,14 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_SSL_VERIFYHOST, 2);
}
-#if LIBCURL_VERSION_NUM >= 0x073100
- if(curl_http_version == 20){
- /* CURL Enable HTTP2*/
- curl_easy_setopt(result,
CURLOPT_HTTP_VERSION,CURL_HTTP_VERSION_2);
- }
+#if LIBCURL_VERSION_NUM >= 0x074700
+// curl_http_version 0 is default.
+if (curl_http_version == 20) {
+ /* Enable HTTP2 when request TLS*/
+ curl_easy_setopt(result,
CURLOPT_HTTP_VERSION,CURL_HTTP_VERSION_2TLS);
+} else if (curl_http_version == 11) {
+ curl_easy_setopt(result,
CURLOPT_HTTP_VERSION,CURL_HTTP_VERSION_1_1);
+}
#endif
#if LIBCURL_VERSION_NUM >= 0x070907
--
gitgitgadget
Re: [PATCH/RFC] Support --append-trailer in cherry-pick and revert
On Thu, Nov 08, 2018 at 09:36:56AM +0900, Junio C Hamano wrote: > Duy Nguyen writes: > > > There is still one thing to settle. "revert -m1" could produce > > something like this > > > > This reverts commit , reversing > > changes made to . > > I do not think it is relevant, with or without multiple parents, to > even attempt to read this message. > > The description is not meant to be machine readable/parseable, but > is meant to be updated to describe the reason why the reversion was > made for human readers. Spending any cycle to attempt interpreting > it by machines will give a wrong signal to encourage people not to > touch it. Instead we should actively encourage people to take that > as the beginning of their description. > > I even suspect that an update to that message to read something like > these > > "This reverts commit because FILL IN THE REASONS HERE" > > "This reverts commit , reversing changes made to >, because FILL IN THE REASONS HERE" Yeah, that was the point I was trying to make earlier today in this thread. I really think of this as a human-readable message. But as far as how this impacts the addition of a trailer: once we have a machine-readable "Reverts:", people naturally may want to know about "Reverts" for older commits. Our options are: 1. say "sorry, there was no machine-readable version back then" 2. try to parse our "This reverts" message and normalize it into "Reverts" for their benefit. Before introducing the new footer, it's worth thinking about the end-game here. If we do (1), then people may want to parse themselves. They are stuck parsing both the old and new (to handle old and new commits). We could make life a little easier on them if we included _both_ the English text and the machine-readable version. And then if they just chose to parse the English, it would work for old and new. But I guess that is really just a losing battle, if we consider the English to be changeable. And doing it ourselves in (2) is really just pushing that losing battle onto ourselves. So if we are comfortable with saying that this is a new feature to have the machine-readable trailer version, and there isn't a robust way to get historical revert information (because there really isn't[1]), then I think we can just punt on any kind of trailer-normalization magic. -Peff [1] Thinking back on reverts I have done, they are often _not_ straight-up reverts. For example, I may end up dropping half of a commit, but leaving some traces of it in place in order to build up the correct solution on top (i.e., fixing whatever problem caused me to revert in the first place). I list those as "this is morally a revert of 1234abcd...", which is definitely not machine readable. ;)
Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()
On Thu, Nov 08, 2018 at 09:30:15AM +0900, Junio C Hamano wrote: > Jeff King writes: > > > On Wed, Nov 07, 2018 at 10:36:52PM +0100, Johannes Sixt wrote: > > > > All that said, if we're just interested in allowing this for config, > > then we already have such a wrapper function: git_config_pathname(). > > > > So I don't think it's a big deal to implement it in any of these ways. > > It's much more important to get the syntax right, because that's > > user-facing and will be with us forever. > > All of us are on the same page after seeing the clarification by > Dscho, it seems. I came to pretty much the same conclusion this > morning before reading this subthread. Outside config values, the > callers of expand_user_path() only feed "~/.git$constant", and they > are all about "personal customization" that do not want to be shared > with other users of the same installation, so "relative to runtime > prefix" feature would not be wanted. But we do not know about new > caller's needs. For now I am OK to have it in expand_user_path(), > possibly renaming the function if people feel it is needed (I don't). I think we would want to carefully think about the call in enter_repo(). We do not want git-daemon to accidentally expose repositories in $RUNTIME_PREFIX. Looking over the code, I think this is OK. The expansion happens in enter_repo(), and then we take the path that was found and do our ok_paths checks on it (which makes sense -- even now you'd ask to export "/home/" and it would need to look at "~peff/repo.git" and expand that to "/home/peff/repo.git" before doing a simple string check. > Between ~ and $VARIABLE_LOOKING_THINGS, I do not have > a strong preference either way, but I am getting an impression that > the latter is more generally favoured in the discussion? I certainly prefer the latter, but I thought I was the only one to have expressed support so far. ;) -Peff
Re: [PATCH 0/1] http: add support selecting http version
On Wed, Nov 07, 2018 at 02:44:51PM +0100, Daniel Stenberg wrote: > On Wed, 7 Nov 2018, Force.Charlie-I via GitGitGadget wrote: > > > Normally, git doesn't need to set curl to select the HTTP version, it > > works fine without HTTP2. Adding HTTP2 support is a icing on the cake. > > Just a FYI: > > Starting with libcurl 7.62.0 (released a week ago), it now defaults to the > "2TLS" setting unless you tell it otherwise. With 2TLS, libcurl will attempt > to use HTTP/2 for HTTPS URLs. With this information, I think I would rather we rely on libcurl to do this rather than putting it in Git. Users will automatically get the best supported protocol instead of having to configure it manually. -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204 signature.asc Description: PGP signature
Re: [PATCH] branch: make --show-current use already resolved HEAD
Rafael Ascensão writes:
> print_current_branch_name() tries to resolve HEAD and die() when it
> doesn't resolve it successfully. But the conditions being tested are
> always unreachable because early in branch:cmd_branch() the same logic
> is performed.
>
> Eliminate the duplicate and unreachable code, and update the current
> logic to the more reliable check for the detached head.
Nice.
> I still think the mention about scripting should be removed from the
> original commit message, leaving it open to being taught other tricks
> like --verbose that aren't necessarily script-friendly.
I'd prefer to see scriptors avoid using "git branch", too.
Unlike end-user facing documentation where we promise "we do X and
will continue to do so because of Y" to the readers, the log message
is primarily for recording the original motivation of the change, so
that we can later learn "we did X back then because we thought Y".
When we want to revise X, we revisit if the reason Y is still valid.
So in that sense, the door to "break" the scriptability is still
open.
> But the main goal of this patch is to just bring some attention to this,
> as I mentioned it in a previous thread but it got lost.
This idea of yours seems to lead to a better implementation, and
indeed "got lost" is a good way to describe what happened---I do not
recall seeing it, for example. Thanks for bringing it back.
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 46f91dc06d..1c51d0a8ca 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -38,6 +38,7 @@ static const char * const builtin_branch_usage[] = {
>
> static const char *head;
> static struct object_id head_oid;
> +static int head_flags = 0;
You've eliminated the "now unnecessary" helper and do everything
inside cmd_branch(), so perhaps this can be made function local, no?
> @@ -668,10 +654,10 @@ int cmd_branch(int argc, const char **argv, const char
> *prefix)
>
> track = git_branch_track;
>
> - head = resolve_refdup("HEAD", 0, _oid, NULL);
> + head = resolve_refdup("HEAD", 0, _oid, _flags);
> if (!head)
> die(_("Failed to resolve HEAD as a valid ref."));
> - if (!strcmp(head, "HEAD"))
> + if (!(head_flags & REF_ISSYMREF))
> filter.detached = 1;
Nice to see we can reuse the resolve_refdup() we already have.
> else if (!skip_prefix(head, "refs/heads/", ))
> die(_("HEAD not found below refs/heads!"));
> @@ -716,7 +702,8 @@ int cmd_branch(int argc, const char **argv, const char
> *prefix)
> die(_("branch name required"));
> return delete_branches(argc, argv, delete > 1, filter.kind,
> quiet);
> } else if (show_current) {
> - print_current_branch_name();
> + if (!filter.detached)
> + puts(head);
Ah, I wondered why we do not have to skip-prefix, but it is already
done for us when we validated that an attached HEAD points at a
local branch. Good.
> return 0;
> } else if (list) {
> /* git branch --local also shows HEAD when it is detached */
Re: [RFC PATCH 3/3] list-objects-filter: teach tree:# how to handle >0
A quick update: I've read Junio's comments on this patchset and
basically agree with them, but I haven't had a chance to apply them
yet. I plan to pick this patchset up again (as well as the patch
"md/list-lazy-objects-fix") once things settle down in my day job.
On Sun, Oct 14, 2018 at 7:31 PM Junio C Hamano wrote:
>
> Matthew DeVore writes:
>
> > The long-term goal at the end of this is to allow a partial clone to
> > eagerly fetch an entire directory of files by fetching a tree and
> > specifying =1. This, for instance, would make a build operation
> > fast and convenient
>
> This would reduce round-trip, as you do not have to fetch the tree
> to see what its contents are before issuing another set of fetches
> for them. Those who are building virtual filesystem that let you
> mount a specific tree object, perhaps via fuse, may find it useful,
> too, even though I suspect that may not be your primary focus.
>
> > diff --git a/Documentation/rev-list-options.txt
> > b/Documentation/rev-list-options.txt
> > index c2c1c40e6..c78985c41 100644
> > --- a/Documentation/rev-list-options.txt
> > +++ b/Documentation/rev-list-options.txt
> > @@ -734,8 +734,12 @@ specification contained in .
> > +
> > The form '--filter=tree:' omits all blobs and trees whose depth
> > from the root tree is >= (minimum depth if an object is located
> > -at multiple depths in the commits traversed). Currently, only =0
> > -is supported, which omits all blobs and trees.
> > +at multiple depths in the commits traversed). =0 will not include
> > +any trees or blobs unless included explicitly in . =1
>
> Here, refers to the objects directly requested on the
> command line (or --stdin)? Triggering this question from me is a
> sign that this description may want to say a bit more to avoid the
> same question from the real readers. Perhaps replace "included
> explicitly in " with "explicitly requested by listing them
> on the command line or feeding them with `--stdin`", or something
> like that?
>
> > diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c
> > index e8da2e858..9dc61d6e6 100644
> > --- a/list-objects-filter-options.c
> > +++ b/list-objects-filter-options.c
> > @@ -50,12 +50,12 @@ static int gently_parse_list_objects_filter(
> > }
> >
> > } else if (skip_prefix(arg, "tree:", )) {
> > - unsigned long depth;
> > - if (!git_parse_ulong(v0, ) || depth != 0) {
> > + if (!git_parse_ulong(v0,
> > +
> > _options->tree_depth_limit_value)) {
> > if (errbuf) {
> > strbuf_addstr(
> > errbuf,
> > - _("only 'tree:0' is supported"));
> > + _("expected 'tree:'"));
>
> We do not accept "tree:-1", even though "-1" is an int. Is it too
> obvious to worry about? I do not think we want to say tree:
> even if we do want to make it clear we reject "tree:-1"
>
> I am wondering if "expected 'tree:'" would work better.
>
> > diff --git a/list-objects-filter-options.h b/list-objects-filter-options.h
> > index af64e5c66..c1ae70cd8 100644
> > --- a/list-objects-filter-options.h
> > +++ b/list-objects-filter-options.h
> > @@ -44,6 +44,7 @@ struct list_objects_filter_options {
> > struct object_id *sparse_oid_value;
> > char *sparse_path_value;
> > unsigned long blob_limit_value;
> > + unsigned long tree_depth_limit_value;
> > };
>
> This change gets it right by adding "depth" in the field name and it
> is not a comment on this patch, but someday not in too distant
> future we should rename the "blob_limit_value" to make it clear that
> it is filtering by number of bytes, as other filtering criteria on
> blobs that can be expressed in ulong are quite possible.
>
> > -static enum list_objects_filter_result filter_trees_none(
> > +static enum list_objects_filter_result filter_trees_depth(
> > enum list_objects_filter_situation filter_situation,
> > struct object *obj,
> > const char *pathname,
> > const char *filename,
> > void *filter_data_)
> > {
> > - struct filter_trees_none_data *filter_data = filter_data_;
> > + struct filter_trees_depth_data *filter_data = filter_data_;
> > +
> > + int too_deep = filter_data->current_depth >= filter_data->max_depth;
>
> Does max mean "maximum allowed", or "this and anything larger are
> rejected". The latter sound wrong, but I offhand do not know if
> your current_depth counts from 0 or 1, so there may not be
> off-by-one.
>
> - dir.c::within_depth() that is used by pathspec matching that in turn
>is used by "grep --max-depth=1" does "if (depth > max_depth)", which
>sounds more in line with the usual convention, I would think.
>
> - pack-objects has max_delta_cache_size, which also is used as
>"maximum allowed", not "this is already too big". So is its
>
Re: [PATCH/RFC] Support --append-trailer in cherry-pick and revert
Duy Nguyen writes: > There is still one thing to settle. "revert -m1" could produce > something like this > > This reverts commit , reversing > changes made to . I do not think it is relevant, with or without multiple parents, to even attempt to read this message. The description is not meant to be machine readable/parseable, but is meant to be updated to describe the reason why the reversion was made for human readers. Spending any cycle to attempt interpreting it by machines will give a wrong signal to encourage people not to touch it. Instead we should actively encourage people to take that as the beginning of their description. I even suspect that an update to that message to read something like these "This reverts commit because FILL IN THE REASONS HERE" "This reverts commit , reversing changes made to , because FILL IN THE REASONS HERE" would be a good idea. It of course is orthogonal to the topic of introducing a new footer to record the "what happened" (without the "why") in a machine-readable way.
Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()
Jeff King writes: > On Wed, Nov 07, 2018 at 10:36:52PM +0100, Johannes Sixt wrote: > > All that said, if we're just interested in allowing this for config, > then we already have such a wrapper function: git_config_pathname(). > > So I don't think it's a big deal to implement it in any of these ways. > It's much more important to get the syntax right, because that's > user-facing and will be with us forever. All of us are on the same page after seeing the clarification by Dscho, it seems. I came to pretty much the same conclusion this morning before reading this subthread. Outside config values, the callers of expand_user_path() only feed "~/.git$constant", and they are all about "personal customization" that do not want to be shared with other users of the same installation, so "relative to runtime prefix" feature would not be wanted. But we do not know about new caller's needs. For now I am OK to have it in expand_user_path(), possibly renaming the function if people feel it is needed (I don't). Between ~ and $VARIABLE_LOOKING_THINGS, I do not have a strong preference either way, but I am getting an impression that the latter is more generally favoured in the discussion?
Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()
Ramsay Jones writes: >> The cute thing is: your absolute paths would not be moved because we are >> talking about Windows. Therefore your absolute paths would not start with >> a forward slash. > > Ah, sorry, I must have misunderstood a comment in your cover letter: > > The reason is this: something like this (make paths specified e.g. via > http.sslCAInfo relative to the runtime prefix) is potentially useful > also in the non-Windows context, as long as Git was built with the > runtime prefix feature. > > ... so I thought you meant to add this code for POSIX systems as well. > > My mistake. :( Well, I do not think you should feel so bad about it, as you are not alone. It wasn't clear to me either that it was to introduce a new syntax that users would not have otherwise used before (i.e. avoids negatively affecting existing settings---because the users would have used a path that begins with a backslash if they meant "full path down from the top of the current drive") to mean a new thing (i.e. "relative to the runtime prefix") that the patch wanted to do. If that is the motivation, then it does make sense to extend this function, and possibly rename it, like this patch does, as we would want this new syntax available in anywhere we take a pathname to an untracked thing. And for such a pathname, we should be consistently using expand_user_path() *anyway* even without this new "now we know how to spell 'relative to the runtime prefix'" feature. So I agree with the patch that the issue it wants to address is worth addressing, and the function to enhance is this one. I am still unsure about the solution, though. I suspect that any build that uses runtime prefix would want access to this feature, regardless of the platform. The new syntax that is "a pathname that begins with a slash is not an absolute path but is relative to the runtime prefix" cannot avoid ambiguity with users' existing settings on platforms other than Windows, which means this feature cannot be made platform neutral in its posted form. The documentation must say "On windows, the way to spell runtime prefix relative paths is this; on macs, you must spell it differently like this." etc. Which is rather unfortunate. Perhaps we need to make an effort to find a syntax that is universally unambiguous and use it consistently across platforms? I am tempted to say "///" might also be such a way, even in the POSIX world, but am not brave enough to do so, as I suspect that may have a fallout in the Windows world X-<. An earlier suggestion by Duy in [*1*] to pretend as if we take $VARIABLE and define special variables might be a better direction. Are there security implications if we started allowing references to environment varibables in strings we pass expand_user_path()? If we cannot convince ourselves that it is safe to allow access to any and all environment variables, then we probably can start by specific "pseudo variables" under our control, like GIT_EXEC_PATH and GIT_INSTALL_ROOT, that do not even have to be tied to environment variables, perhaps with further restriction to allow it only at the beginning of the string, or something like that, if necessary. [References] *1*
Re: What exactly is a "initial checkout"
On 07/11/2018 08:50, Christian Halstrick wrote: Ok, I know understand the problems which are solved by this special behaviour of a "initial checkout". And also important I understand when exactly I should do a "initial checkout" - when the index file does not exist. I'll share my new knowledge with JGit :-) Given that the initial query was about the lack of documentation for the term "initial checkout", do you have any suggestion of how it might best be incorporated into the documentation to assist future reader? -- Philip
[PATCH] branch: make --show-current use already resolved HEAD
print_current_branch_name() tries to resolve HEAD and die() when it
doesn't resolve it successfully. But the conditions being tested are
always unreachable because early in branch:cmd_branch() the same logic
is performed.
Eliminate the duplicate and unreachable code, and update the current
logic to the more reliable check for the detached head.
Signed-off-by: Rafael Ascensão
---
This patch is meant to be either applied or squashed on top of the
current series.
I am basing the claims of it being more reliable of what Junio suggested
on a previous iteration of this series:
https://public-inbox.org/git/xmqq4ldtgehs@gitster-ct.c.googlers.com/
But the main goal of this patch is to just bring some attention to this,
as I mentioned it in a previous thread but it got lost. After asking on
#git-devel, the suggestion was to send it as an incremental patch. So
here it is. :)
I still think the mention about scripting should be removed from the
original commit message, leaving it open to being taught other tricks
like --verbose that aren't necessarily script-friendly.
Cheers
builtin/branch.c | 23 +--
1 file changed, 5 insertions(+), 18 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 46f91dc06d..1c51d0a8ca 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -38,6 +38,7 @@ static const char * const builtin_branch_usage[] = {
static const char *head;
static struct object_id head_oid;
+static int head_flags = 0;
static int branch_use_color = -1;
static char branch_colors[][COLOR_MAXLEN] = {
@@ -443,21 +444,6 @@ static void print_ref_list(struct ref_filter *filter,
struct ref_sorting *sortin
free(to_free);
}
-static void print_current_branch_name(void)
-{
- int flags;
- const char *refname = resolve_ref_unsafe("HEAD", 0, NULL, );
- const char *shortname;
- if (!refname)
- die(_("could not resolve HEAD"));
- else if (!(flags & REF_ISSYMREF))
- return;
- else if (skip_prefix(refname, "refs/heads/", ))
- puts(shortname);
- else
- die(_("HEAD (%s) points outside of refs/heads/"), refname);
-}
-
static void reject_rebase_or_bisect_branch(const char *target)
{
struct worktree **worktrees = get_worktrees(0);
@@ -668,10 +654,10 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
track = git_branch_track;
- head = resolve_refdup("HEAD", 0, _oid, NULL);
+ head = resolve_refdup("HEAD", 0, _oid, _flags);
if (!head)
die(_("Failed to resolve HEAD as a valid ref."));
- if (!strcmp(head, "HEAD"))
+ if (!(head_flags & REF_ISSYMREF))
filter.detached = 1;
else if (!skip_prefix(head, "refs/heads/", ))
die(_("HEAD not found below refs/heads!"));
@@ -716,7 +702,8 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
die(_("branch name required"));
return delete_branches(argc, argv, delete > 1, filter.kind,
quiet);
} else if (show_current) {
- print_current_branch_name();
+ if (!filter.detached)
+ puts(head);
return 0;
} else if (list) {
/* git branch --local also shows HEAD when it is detached */
--
2.19.1
Re: [RFC PATCH] index-pack: improve performance on NFS
On Mon, Oct 29, 2018 at 07:27:39PM -0400, Jeff King wrote: > On Mon, Oct 29, 2018 at 08:36:07PM +0100, Ævar Arnfjörð Bjarmason wrote: > > * Re-roll my 4 patch series to include the patch you have in > ><20181027093300.ga23...@sigill.intra.peff.net> > > I don't think it's quite ready for inclusion as-is. I hope to brush it > up a bit, but I have quite a backlog of stuff to review, as well. We're still quite keen to get this patch included. Is there anything I can do to help? Also I just re-read your comments on maximum cache size. I think you were arguing both sides of the equation and I wasn't sure where you'd ended up. :) A larger cache size potentially takes more time to fill up especially on NFS while a smaller cache size obviously would less effective. That said a small cache is still effective for the "clone" case where the repo is empty. It also occurred to me that as a performance optimization your patch could read the the loose object directories in parallel using a thread pool. At least on Amazon EFS this should result in al almost linear performance increase. I'm not sure how much this would help for local file systems. In any case this may be best done as a follow-up patch (that I'd be happy to volunteer for).
Re: [PATCH v3 1/2] range-diff doc: add a section about output stability
Stephen & Linda Smith writes: >> +This is particularly true when passing in diff options. Currently some >> +options like `--stat` can as an emergent effect produce output that's > > "`--stat` can as an emergent": I read that for times to decided it was > correct > grammar. Should it be reworded to read better? Just a nit. A pair of comma around "as an ... effect" would make it a bit more readable. It also took me four reads before I convinced myself that the original wants to say "Some options may just do whatever they happen to do that result in pretty useless output". > >> +quite useless in the context of `range-diff`. Future versions of >> +`range-diff` may learn to interpret such options in a manner specifc >> +to `range-diff` (e.g. for `--stat` summarizing how the diffstat >> +changed).
Re: [PATCH v2] format-patch: respect --stat in cover letter's diffstat
On 11/07/18 17:49, Nguyễn Thái Ngọc Duy wrote:
> Commit 43662b23ab (format-patch: keep cover-letter diffstat wrapped in
> 72 columns - 2018-01-24) uncondtionally sets stat width to 72 when
> generating diffstat for the cover letter, ignoring --stat from command
> line. But it should only do so when stat width is still default
> (i.e. stat_width == 0).
>
> In order to fix this, we should only set stat_width if stat_width is
> zero. But it will never be. Commit 071dd0ba43 (format-patch: reduce
> patch diffstat width to 72 - 2018-02-01) makes sure that default stat
> width will be 72 (ignoring $COLUMNS, but could still be overriden by
> --stat). So all we need to do here is drop the assignment.
>
> Reported-by: Laszlo Ersek
> Helped-by: Leif Lindholm
> Signed-off-by: Nguyễn Thái Ngọc Duy
> ---
> builtin/log.c | 2 --
> t/t4052-stat-output.sh | 48 +-
> 2 files changed, 33 insertions(+), 17 deletions(-)
>
> diff --git a/builtin/log.c b/builtin/log.c
> index 061d4fd864..1a39c6e52a 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -1009,8 +1009,6 @@ static void show_diffstat(struct rev_info *rev,
>
> memcpy(, >diffopt, sizeof(opts));
> opts.output_format = DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;
> - opts.stat_width = MAIL_DEFAULT_WRAP;
> -
> diff_setup_done();
>
> diff_tree_oid(get_commit_tree_oid(origin),
Because I plan to use the patch on top of v2.19.1 (until the next major
release, v2.20, is made), that's also where I applied and tested the patch.
With master @ a4b8ab5363a3, this patch targets show_diffstat(). At
v2.19.1, commit fa5b7ea670f4 ("format-patch: allow additional generated
content in make_cover_letter()", 2018-07-23) had not occurred yet, so
there the subject code still lived in make_cover_letter(). On my end,
git-am has applied the hunk to make_cover_letter() seamlessly.
I tested the patch with "--stat=1000 --stat-graph-width=20", formatting
an edk2 series that contained commit 1ed6498c4a02
("UefiCpuPkg/CommonFeature: Skip locking when the feature is disabled",
2018-11-07). The long pathname
"UefiCpuPkg/Library/CpuCommonFeaturesLib/FeatureControl.c" is no longer
truncated in the cumulative diffstat, in the cover letter.
Tested-by: Laszlo Ersek
> diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh
> [...]
I didn't try to run the test suite (I wasn't conscious of it anyway); I
built & installed git with
nice make -j4 prefix=... all doc info
nice make prefix=... install install-doc install-html install-info
I also wasn't watching the make log. So if those make targets don't
include the test suite, then I didn't exercise the new test case.
Thank you!
Laszlo
Re: What's cooking in git.git (Nov 2018, #03; Wed, 7)
Duy Nguyen writes:
> On Wed, Nov 7, 2018 at 2:09 PM SZEDER Gábor wrote:
>>
>> On Wed, Nov 07, 2018 at 06:41:45PM +0900, Junio C Hamano wrote:
>> > * nd/i18n (2018-11-06) 16 commits
>> > - fsck: mark strings for translation
>> > - fsck: reduce word legos to help i18n
>> > ...
>> > More _("i18n") markings.
>>
>> When this patch is merged into 'pu' all four tests added to
>> 't1450-fsck.sh' in b29759d89a (fsck: check HEAD and reflog from other
>> worktrees, 2018-10-21) as part of 'nd/per-worktree-ref-iteration'
>> below fail when run with GETTEXT_POISON=y. The test suite passes in
>> both of these topics on their own, even with GETTEXT_POISON, it's
>> their merge that is somehow problematic.
>
> Not surprising. The i18n series makes fsck output localized strings
> and without updating grep to test_i18ngrep, new tests will fail. If
> 'pu' was passing before, I'm ok with just ejecting this series for
> now. Then I wait for the other to land, rebase, fixup and resubmit.
Let me first see if I can come up with a merge-fix that can be
carried around during the time this topic cooks, before talking
about dropping and reattempting the series.
For a change like this, a time-window in which the codebase is
quiescent enough may never come, and because the changes go all over
the place, mostly but not entirely repetitive, it costs a lot, not
just to write but also to review them.
Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()
On Wed, Nov 07, 2018 at 10:36:52PM +0100, Johannes Sixt wrote:
> Am 07.11.18 um 21:41 schrieb Jeff King:
> > On Wed, Nov 07, 2018 at 07:52:28PM +0100, Johannes Sixt wrote:
> > > Do I understand correctly, that you use a leading slash as an indicator to
> > > construct a path relative to system_path(). How about a "reserved" user
> > > name? For example,
> > >
> > >[http] sslcert = ~system_path/what/ever
> > >
> > > although a more unique name, perhaps with some punctuation, may be
> > > desirable.
> >
> > It's syntactically a bit further afield, but something like:
> >
> >[http]
> >sslcert = $RUNTIME_PREFIX/what/ever
> >
> > would make sense to me, and is a bit less subtle than the fake user. I
> > don't know if that would confuse people into thinking that we
> > interpolate arbitrary environment variables, though.
>
> The expansion of a fake user name would have to go in expand_user_path(), a
> fake variable name would have to be expanded elsewhere. Now, Dscho mentions
> in the cover letter that his patch has already seen some field testing ("has
> been 'in production' for a good while"). So, we have gained some confidence
> that the point where the substitution happens, in expand_user_path(), is
> suitable. Therefore, I have slight preference for a fake user.
I don't think that necessarily needs to limit us. expand_user_path() is
named that because right now it just expands "~". But there's no reason
it couldn't be used for general expansion. The problem in the original
patch is that it expands _even when the user has not asked us to do it_.
Looking at the callers, most of them would be fine with the new
expansion (the only exception is the one in daemon.c, though it manually
checks for "~" already).
Now I agree that the new function would probably need a better name, at
which point you could easily have a function expand_path() which just
wraps expand_user_path().
All that said, if we're just interested in allowing this for config,
then we already have such a wrapper function: git_config_pathname().
So I don't think it's a big deal to implement it in any of these ways.
It's much more important to get the syntax right, because that's
user-facing and will be with us forever.
-Peff
You emailed AcroArts Productions
Name: you開﹤普.专。票﹥，电/微：134//186//l8⑼//58，长=期=有=效 Email: git@vger.kernel.org Comments: you開﹤普.专。票﹥，电/微：134//186//l8⑼//58，长=期=有=效
Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()
Am 07.11.18 um 21:41 schrieb Jeff King:
On Wed, Nov 07, 2018 at 07:52:28PM +0100, Johannes Sixt wrote:
Do I understand correctly, that you use a leading slash as an indicator to
construct a path relative to system_path(). How about a "reserved" user
name? For example,
[http] sslcert = ~system_path/what/ever
although a more unique name, perhaps with some punctuation, may be
desirable.
It's syntactically a bit further afield, but something like:
[http]
sslcert = $RUNTIME_PREFIX/what/ever
would make sense to me, and is a bit less subtle than the fake user. I
don't know if that would confuse people into thinking that we
interpolate arbitrary environment variables, though.
The expansion of a fake user name would have to go in
expand_user_path(), a fake variable name would have to be expanded
elsewhere. Now, Dscho mentions in the cover letter that his patch has
already seen some field testing ("has been 'in production' for a good
while"). So, we have gained some confidence that the point where the
substitution happens, in expand_user_path(), is suitable. Therefore, I
have slight preference for a fake user.
-- Hannes
Re: [PATCH/RFC] Support --append-trailer in cherry-pick and revert
On Wed, Nov 07, 2018 at 04:30:38PM +0100, Duy Nguyen wrote: > > Could we help the reading scripts by normalizing old and new output via > > interpret-trailers, %(trailers), etc? > > > > I think "(cherry picked from ...)" is already considered a trailer by > > the trailer code. If the caller instructs us to, we could probably > > rewrite it to: > > > > Cherry-picked-from: ... > > > > in the output. Then the end-game is that scripts should just use > > interpret-trailers, etc, and old and new commits will Just Work. > > There is still one thing to settle. "revert -m1" could produce > something like this > > This reverts commit , reversing > changes made to . > > My proposal produces this > > Reverts: ^2 > > And I can't really convert the former to latter without accessing > object database (probably not a good idea?) to check if SHA2 is the > second parent of SHA1. So either > > - I access object database anyway > - Generate just "Reverts: " (i.e. losing info) with interpret-trailers > - Change Reverts: tag to a different output format, or maybe use two > tags instead. IMHO the revert case is way less interesting for automated parsing. In a workflow like Git's, cherry-picks aren't very common, but there _are_ workflows where there's a lot of cherry-picking between dev/release branches, and automated analysis is useful there. Whereas for revert, it's almost always a human-scale thing. A commit was bad, so you revert it. The annotation is useful if you're digging, but it's not generally going to be a fundamental part of a workflow. And it's not really any different than fixing a bug later. And I think that's reflected in the way we just casually stick the reverted oid in the human-readable part of the commit message (and the lack of any tools to parse it). So IMHO it would be OK to treat this less carefully than the cherry-pick case. -Peff
Re: [PATCH 2/2] built-in rebase --autostash: leave the current branch alone if possible
On Wed, Nov 07, 2018 at 06:00:50AM -0800, Johannes Schindelin via GitGitGadget wrote: > From: Johannes Schindelin > > When we converted a `git reset --hard` call in the original Unix shell > script to built-in code, we asked to reset the worktree and the index > and explicitly *not* to detach the HEAD. By mistake, though, we still > did. Let's fix this. > [...] > diff --git a/builtin/rebase.c b/builtin/rebase.c > index 0ee06aa363..4a608d0a78 100644 > --- a/builtin/rebase.c > +++ b/builtin/rebase.c > @@ -613,7 +613,8 @@ static int reset_head(struct object_id *oid, const char > *action, > reflog_head = msg.buf; > } > if (!switch_to_branch) > - ret = update_ref(reflog_head, "HEAD", oid, orig, REF_NO_DEREF, > + ret = update_ref(reflog_head, "HEAD", oid, orig, > + detach_head ? REF_NO_DEREF : 0, >UPDATE_REFS_MSG_ON_ERR); This makes sense. There are actually a bunch of calls that pass detach_head==0, besides the one related to autostash. I suspect for most of them it does not matter, because either: 1. We are already on a detached HEAD, since we detach as the first step of the rebase. So for the call in ACTION_SKIP, for example, we probably cannot trigger the problem. 2. They pass a switch_to_branch arg, so we do not hit this code path anyway (the call to fast-forward is like this, for example). So there may be other ways to trigger the problem, but I didn't dig. Either way, your fix is clearly the right thing to do. -Peff
Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()
On Wed, Nov 07, 2018 at 07:52:28PM +0100, Johannes Sixt wrote: > > Okay, now we know everything you find wrong with the current patch. Do you > > have any suggestion how to make it right? I.e. what would you suggest as a > > way to specify in a gitconfig in a portable Git where the certificate > > bundle is? > > Ah, so your actual problem is quite a different one! > > Do I understand correctly, that you use a leading slash as an indicator to > construct a path relative to system_path(). How about a "reserved" user > name? For example, > > [http] sslcert = ~system_path/what/ever > > although a more unique name, perhaps with some punctuation, may be > desirable. It's syntactically a bit further afield, but something like: [http] sslcert = $RUNTIME_PREFIX/what/ever would make sense to me, and is a bit less subtle than the fake user. I don't know if that would confuse people into thinking that we interpolate arbitrary environment variables, though. -Peff
Attention.
Compliments of the day ; I have a business proposition for you which I sent you previously,I do not know if you received it? Please do find it proper to write me if your email is still active. Yours Faithfully, Barr. Alexander Stewart.
Re: [PATCH v3 1/2] range-diff doc: add a section about output stability
On Wed, 7 Nov 2018 at 13:22, Ævar Arnfjörð Bjarmason wrote: > + > +This is particularly true when passing in diff options. Currently some > +options like `--stat` can as an emergent effect produce output that's > +quite useless in the context of `range-diff`. Future versions of > +`range-diff` may learn to interpret such options in a manner specifc s/specifc/specific/ > +to `range-diff` (e.g. for `--stat` summarizing how the diffstat > +changed).
Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()
Am 07.11.18 um 12:23 schrieb Johannes Schindelin: On Tue, 6 Nov 2018, Johannes Sixt wrote: Am 06.11.18 um 15:53 schrieb Johannes Schindelin via GitGitGadget: Even if a path looks like a POSIX paths, i.e. it starts with a directory separator, but not with drive-letter-colon, it still has a particular meaning, namely (as far as I know) that the path is anchored at the root of the drive of the current working directory. If a user specifies such a path on Windows, and it must undergo expand_user_path(), then that is a user error, or the user knows what they are doing. I think it is wrong to interpret such a path as relative to some random other directory, like this patch seems to do. Okay, now we know everything you find wrong with the current patch. Do you have any suggestion how to make it right? I.e. what would you suggest as a way to specify in a gitconfig in a portable Git where the certificate bundle is? Ah, so your actual problem is quite a different one! Do I understand correctly, that you use a leading slash as an indicator to construct a path relative to system_path(). How about a "reserved" user name? For example, [http] sslcert = ~system_path/what/ever although a more unique name, perhaps with some punctuation, may be desirable. -- Hannes
Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()
On 07/11/2018 11:19, Johannes Schindelin wrote: [snip] >>> Hmm, this doesn't quite fit with the intended use of this >>> function! ;-) (even on windows!) >>> >>> I haven't looked very deeply, but doesn't this affect all >>> absolute paths in the config read by git_config_pathname(), >>> along with all 'included config' files? >> >> I think so. I have not thought things through to say if replacing a >> "full path in the current drive" with system_path() is a sensible >> thing to do in the first place, but I am getting the impression from >> review comments that it probably is not. >> >>> I am pretty sure that I would not want the absolute paths >>> in my config file(s) magically 'moved' depending on whether >>> git has been compiled with 'runtime prefix' support or not! > > The cute thing is: your absolute paths would not be moved because we are > talking about Windows. Therefore your absolute paths would not start with > a forward slash. Ah, sorry, I must have misunderstood a comment in your cover letter: The reason is this: something like this (make paths specified e.g. via http.sslCAInfo relative to the runtime prefix) is potentially useful also in the non-Windows context, as long as Git was built with the runtime prefix feature. ... so I thought you meant to add this code for POSIX systems as well. My mistake. :( ATB, Ramsay Jones
[PATCH v2] format-patch: respect --stat in cover letter's diffstat
Commit 43662b23ab (format-patch: keep cover-letter diffstat wrapped in 72 columns - 2018-01-24) uncondtionally sets stat width to 72 when generating diffstat for the cover letter, ignoring --stat from command line. But it should only do so when stat width is still default (i.e. stat_width == 0). In order to fix this, we should only set stat_width if stat_width is zero. But it will never be. Commit 071dd0ba43 (format-patch: reduce patch diffstat width to 72 - 2018-02-01) makes sure that default stat width will be 72 (ignoring $COLUMNS, but could still be overriden by --stat). So all we need to do here is drop the assignment. Reported-by: Laszlo Ersek Helped-by: Leif Lindholm Signed-off-by: Nguyễn Thái Ngọc Duy --- builtin/log.c | 2 -- t/t4052-stat-output.sh | 48 +- 2 files changed, 33 insertions(+), 17 deletions(-) diff --git a/builtin/log.c b/builtin/log.c index 061d4fd864..1a39c6e52a 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1009,8 +1009,6 @@ static void show_diffstat(struct rev_info *rev, memcpy(, >diffopt, sizeof(opts)); opts.output_format = DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT; - opts.stat_width = MAIL_DEFAULT_WRAP; - diff_setup_done(); diff_tree_oid(get_commit_tree_oid(origin), diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh index 6e2cf933f7..b1ce0d9b97 100755 --- a/t/t4052-stat-output.sh +++ b/t/t4052-stat-output.sh @@ -44,42 +44,50 @@ show --stat log -1 --stat EOF -while read cmd args +cat >expect.40 <<-'EOF' + ...a | 1 + +EOF +cat >expect.6030 <<-'EOF' + ...aaa | 1 + +EOF +cat >expect2.40 <<-'EOF' + ...a | 1 + + ...a | 1 + +EOF +cat >expect2.6030 <<-'EOF' + ...aaa | 1 + + ...aaa | 1 + +EOF +while read expect cmd args do - cat >expect <<-'EOF' -...a | 1 + - EOF test_expect_success "$cmd --stat=width: a long name is given more room when the bar is short" ' git $cmd $args --stat=40 >output && grep " | " output >actual && - test_cmp expect actual + test_cmp $expect.40 actual ' test_expect_success "$cmd --stat-width=width with long name" ' git $cmd $args --stat-width=40 >output && grep " | " output >actual && - test_cmp expect actual + test_cmp $expect.40 actual ' - cat >expect <<-'EOF' -...aaa | 1 + - EOF test_expect_success "$cmd --stat=...,name-width with long name" ' git $cmd $args --stat=60,30 >output && grep " | " output >actual && - test_cmp expect actual + test_cmp $expect.6030 actual ' test_expect_success "$cmd --stat-name-width with long name" ' git $cmd $args --stat-name-width=30 >output && grep " | " output >actual && - test_cmp expect actual + test_cmp $expect.6030 actual ' done <<\EOF -format-patch -1 --stdout -diff HEAD^ HEAD --stat -show --stat -log -1 --stat +expect2 format-patch --cover-letter -1 --stdout +expect diff HEAD^ HEAD --stat +expect show --stat +expect log -1 --stat EOF @@ -95,6 +103,16 @@ test_expect_success 'preparation for big change tests' ' git commit -m message abcd ' +cat >expect72 <<'EOF' + abcd | 1000 ++ + abcd | 1000 ++ +EOF +test_expect_success "format-patch --cover-letter ignores COLUMNS (big change)" ' + COLUMNS=200 git format-patch -1 --stdout --cover-letter >output && + grep " | " output >actual && + test_cmp expect72 actual +' + cat >expect72 <<'EOF' abcd | 1000 ++ EOF -- 2.19.1.1005.gac84295441
Re: [PATCH/RFC] Support --append-trailer in cherry-pick and revert
On Tue, Nov 6, 2018 at 11:11 PM Jeff King wrote: > > On Tue, Nov 06, 2018 at 06:48:22PM +0100, Ævar Arnfjörð Bjarmason wrote: > > > The implementation looks fine to me, but as noted in > > https://public-inbox.org/git/8736se6qyc@evledraar.gmail.com/ I > > wonder what the plausible end-game is. That we'll turn this on by > > default in a few years, and then you can only worry about > > git-interpret-trailers for repos created as of 2020 or something? > > > > Otherwise it seems we'll need to *also* parse out the existing messages > > we've added. > > Could we help the reading scripts by normalizing old and new output via > interpret-trailers, %(trailers), etc? > > I think "(cherry picked from ...)" is already considered a trailer by > the trailer code. If the caller instructs us to, we could probably > rewrite it to: > > Cherry-picked-from: ... > > in the output. Then the end-game is that scripts should just use > interpret-trailers, etc, and old and new commits will Just Work. There is still one thing to settle. "revert -m1" could produce something like this This reverts commit , reversing changes made to . My proposal produces this Reverts: ^2 And I can't really convert the former to latter without accessing object database (probably not a good idea?) to check if SHA2 is the second parent of SHA1. So either - I access object database anyway - Generate just "Reverts: " (i.e. losing info) with interpret-trailers - Change Reverts: tag to a different output format, or maybe use two tags instead. -- Duy
Re: What's cooking in git.git (Nov 2018, #03; Wed, 7)
On Wed, Nov 7, 2018 at 2:09 PM SZEDER Gábor wrote:
>
> On Wed, Nov 07, 2018 at 06:41:45PM +0900, Junio C Hamano wrote:
> > * nd/i18n (2018-11-06) 16 commits
> > - fsck: mark strings for translation
> > - fsck: reduce word legos to help i18n
> > - parse-options.c: mark more strings for translation
> > - parse-options.c: turn some die() to BUG()
> > - parse-options: replace opterror() with optname()
> > - repack: mark more strings for translation
> > - remote.c: mark messages for translation
> > - remote.c: turn some error() or die() to BUG()
> > - reflog: mark strings for translation
> > - read-cache.c: add missing colon separators
> > - read-cache.c: mark more strings for translation
> > - read-cache.c: turn die("internal error") to BUG()
> > - attr.c: mark more string for translation
> > - archive.c: mark more strings for translation
> > - alias.c: mark split_cmdline_strerror() strings for translation
> > - git.c: mark more strings for translation
> >
> > More _("i18n") markings.
>
> When this patch is merged into 'pu' all four tests added to
> 't1450-fsck.sh' in b29759d89a (fsck: check HEAD and reflog from other
> worktrees, 2018-10-21) as part of 'nd/per-worktree-ref-iteration'
> below fail when run with GETTEXT_POISON=y. The test suite passes in
> both of these topics on their own, even with GETTEXT_POISON, it's
> their merge that is somehow problematic.
Not surprising. The i18n series makes fsck output localized strings
and without updating grep to test_i18ngrep, new tests will fail. If
'pu' was passing before, I'm ok with just ejecting this series for
now. Then I wait for the other to land, rebase, fixup and resubmit.
--
Duy
Git Test Coverage Report (Wednesday, Nov 7)
Here is the coverage report for today.
Thanks,
-Stolee
[1] https://dev.azure.com/git/git/_build/results?buildId=251=logs
---
pu: 381b31f0006e46fe041e7fc6e5f7b19da5ccd889
jch: ab76604d6537afa18c9d8588c08f699c1f539659
next: 8438c0b2453a7207c9c45756f5e37dfe283db602
master: 8858448bb49332d353febc078ce4a3abcc962efe
master@{1}: d582ea202b626dcc6c3b01e1e11a296d9badd730
Uncovered code in 'pu' not in 'jch'
--
builtin/blame.c
381b31f000 builtin/blame.c 200)
repo_unuse_commit_buffer(the_repository, commit, message);
74e8221b52 builtin/blame.c 928) blame_date_width = sizeof("Thu Oct 19
16:00");
74e8221b52 builtin/blame.c 929) break;
builtin/describe.c
381b31f000 builtin/describe.c 257) repo_parse_commit(the_repository, p);
builtin/pack-objects.c
381b31f000 builtin/pack-objects.c 2832) if
(!repo_has_object_file(the_repository, >oid) &&
is_promisor_object(>oid))
date.c
74e8221b52 113) die("Timestamp too large for this system: %"PRItime, time);
74e8221b52 216) if (tm->tm_mon == human_tm->tm_mon) {
74e8221b52 217) if (tm->tm_mday > human_tm->tm_mday) {
74e8221b52 219) } else if (tm->tm_mday == human_tm->tm_mday) {
74e8221b52 220) hide.date = hide.wday = 1;
74e8221b52 221) } else if (tm->tm_mday + 5 > human_tm->tm_mday) {
74e8221b52 223) hide.date = 1;
74e8221b52 231) gettimeofday(, NULL);
74e8221b52 232) show_date_relative(time, tz, , buf);
74e8221b52 233) return;
74e8221b52 246) hide.seconds = 1;
74e8221b52 247) hide.tz |= !hide.date;
74e8221b52 248) hide.wday = hide.time = !hide.year;
74e8221b52 262) strbuf_rtrim(buf);
74e8221b52 287) gettimeofday(, NULL);
74e8221b52 290) human_tz = local_time_tzoffset(now.tv_sec, _tm);
74e8221b52 886) static int auto_date_style(void)
74e8221b52 888) return (isatty(1) || pager_in_use()) ? DATE_HUMAN :
DATE_NORMAL;
74e8221b52 909) return DATE_HUMAN;
74e8221b52 911) return auto_date_style();
diff.c
b613de67c4 316) ret |= COLOR_MOVED_WS_ERROR;
b613de67c4 348) unsigned cm = parse_color_moved_ws(value);
b613de67c4 349) if (cm & COLOR_MOVED_WS_ERROR)
fast-import.c
381b31f000 2935) buf = repo_read_object_file(the_repository, oid, ,
);
381b31f000 3041) buf = repo_read_object_file(the_repository, oid, ,
fsck.c
381b31f000 858) repo_unuse_commit_buffer(the_repository, commit, buffer);
381b31f000 878) repo_read_object_file(the_repository,
381b31f000 879) >object.oid, , );
http-push.c
381b31f000 1635) if (!repo_has_object_file(the_repository, _oid))
381b31f000 1642) if (!repo_has_object_file(the_repository,
_ref->old_oid))
negotiator/default.c
381b31f000 71) if (repo_parse_commit(the_repository, commit))
remote.c
879b6a9e6f 1140) return error(_("dst ref %s receives from more than one
src."),
revision.c
381b31f000 726) if (repo_parse_commit(the_repository, p) < 0)
sequencer.c
381b31f000 1624) repo_unuse_commit_buffer(the_repository, head_commit,
381b31f000 3868) repo_unuse_commit_buffer(the_repository,
sha1-array.c
bba406749a 91) oidcpy([dst], [src]);
submodule.c
b303ef65e7 524) the_repository->submodule_prefix :
e2419f7e30 1378) strbuf_release();
7454fe5cb6 1501) struct get_next_submodule_task *task = task_cb;
7454fe5cb6 1505) get_next_submodule_task_release(task);
7454fe5cb6 1532) return 0;
7454fe5cb6 1536) goto out;
7454fe5cb6 1551) return 0;
tree.c
381b31f000 108) if (repo_parse_commit(the_repository, commit))
wrapper.c
5efde212fc 70) die("Out of memory, malloc failed (tried to allocate %"
PRIuMAX " bytes)",
5efde212fc 73) error("Out of memory, malloc failed (tried to allocate
%" PRIuMAX " bytes)",
Commits introducing uncovered code:
Ævar Arnfjörð Bjarmason 879b6a9e6: i18n: remote.c: mark error(...)
messages for translation
Junio C Hamano 381b31f00: treewide: apply cocci patch
Linus Torvalds 74e8221b5: Add 'human' date format
Martin Koegler 5efde212f: zlib.c: use size_t for size
Stefan Beller 7454fe5cb: fetch: try fetching submodules if needed
objects were not fetched
Stefan Beller b303ef65e: submodule: use submodule repos for object
lookup
Stefan Beller b613de67c: diff: differentiate error handling in
parse_color_moved_ws
Stefan Beller bba406749: sha1-array: provide oid_array_filter
Stefan Beller e2419f7e3: submodule: migrate get_next_submodule to
use repository structs
Uncovered code in 'jch' not in 'next'
apply.c
517fe807d6 4776) BUG_ON_OPT_NEG(unset);
735ca208c5 4830) return -1;
archive.c
8a705c4638 399) die(_("not a valid object name: %s"), name);
8a705c4638 412) die(_("not a tree object: %s"), oid_to_hex());
8a705c4638 422) die(_("current working directory is untracked"));
attr.c
aa4fa3fa79 369) fprintf_ln(stderr, _("%s not allowed: %s:%d"),
builtin/am.c
fce5664805 2117) *opt_value = PATCH_FORMAT_UNKNOWN;
builtin/blame.c
517fe807d6 builtin/blame.c 759) BUG_ON_OPT_NEG(unset);
builtin/branch.c
0ecb1fc726 builtin/branch.c 452) die(_("could not resolve HEAD"));
[PATCH 0/2] built-in rebase --autostash: fix regression
It was reported that the new, shiny built-in git rebase has a bug where it would detach the HEAD when it was not even necessary to detach it. Keeping with my fine tradition to first demonstrate what is the actual problem (and making it easy to verify my claim), this patch series first introduces the regression test, and then the (quite simple) fix. AEvar, sorry for the ASCII-fication of your name, I still did not find the time to look at the GitGitGadget bug closely where it does the wrong thing when Cc:ing with non-ASCII names. Johannes Schindelin (2): built-in rebase: demonstrate regression with --autostash built-in rebase --autostash: leave the current branch alone if possible builtin/rebase.c| 3 ++- t/t3420-rebase-autostash.sh | 8 2 files changed, 10 insertions(+), 1 deletion(-) base-commit: 8858448bb49332d353febc078ce4a3abcc962efe Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-70%2Fdscho%2Ffix-built-in-rebase-autostash-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-70/dscho/fix-built-in-rebase-autostash-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/70 -- gitgitgadget
[PATCH 1/2] built-in rebase: demonstrate regression with --autostash
From: Johannes Schindelin An unnamed colleague of Ævar Arnfjörð Bjarmason reported a breakage where a `pull --rebase` (which did not really need to do anything but stash, see that nothing was changed, and apply the stash again) also detached the HEAD. This patch adds a minimal reproducer for this regression. Signed-off-by: Johannes Schindelin --- t/t3420-rebase-autostash.sh | 8 1 file changed, 8 insertions(+) diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh index f355c6825a..d4e2520bcb 100755 --- a/t/t3420-rebase-autostash.sh +++ b/t/t3420-rebase-autostash.sh @@ -361,4 +361,12 @@ test_expect_success 'autostash with dirty submodules' ' git rebase -i --autostash HEAD ' +test_expect_failure 'branch is left alone when possible' ' + git checkout -b unchanged-branch && + echo changed >file0 && + git rebase --autostash unchanged-branch && + test changed = "$(cat file0)" && + test unchanged-branch = "$(git rev-parse --abbrev-ref HEAD)" +' + test_done -- gitgitgadget
[PATCH 2/2] built-in rebase --autostash: leave the current branch alone if possible
From: Johannes Schindelin
When we converted a `git reset --hard` call in the original Unix shell
script to built-in code, we asked to reset the worktree and the index
and explicitly *not* to detach the HEAD. By mistake, though, we still
did. Let's fix this.
Signed-off-by: Johannes Schindelin
---
builtin/rebase.c| 3 ++-
t/t3420-rebase-autostash.sh | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 0ee06aa363..4a608d0a78 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -613,7 +613,8 @@ static int reset_head(struct object_id *oid, const char
*action,
reflog_head = msg.buf;
}
if (!switch_to_branch)
- ret = update_ref(reflog_head, "HEAD", oid, orig, REF_NO_DEREF,
+ ret = update_ref(reflog_head, "HEAD", oid, orig,
+detach_head ? REF_NO_DEREF : 0,
UPDATE_REFS_MSG_ON_ERR);
else {
ret = create_symref("HEAD", switch_to_branch, msg.buf);
diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh
index d4e2520bcb..4c7494cc8f 100755
--- a/t/t3420-rebase-autostash.sh
+++ b/t/t3420-rebase-autostash.sh
@@ -361,7 +361,7 @@ test_expect_success 'autostash with dirty submodules' '
git rebase -i --autostash HEAD
'
-test_expect_failure 'branch is left alone when possible' '
+test_expect_success 'branch is left alone when possible' '
git checkout -b unchanged-branch &&
echo changed >file0 &&
git rebase --autostash unchanged-branch &&
--
gitgitgadget
Re: [PATCH 0/1] http: add support selecting http version
On Wed, 7 Nov 2018, Force.Charlie-I via GitGitGadget wrote: Normally, git doesn't need to set curl to select the HTTP version, it works fine without HTTP2. Adding HTTP2 support is a icing on the cake. Just a FYI: Starting with libcurl 7.62.0 (released a week ago), it now defaults to the "2TLS" setting unless you tell it otherwise. With 2TLS, libcurl will attempt to use HTTP/2 for HTTPS URLs. -- / daniel.haxx.se
Re: Regression in rebase-in-C with rebase.autoStash=true
Hi Ævar,
On Tue, 6 Nov 2018, Ævar Arnfjörð Bjarmason wrote:
>
> On Tue, Oct 23 2018, Johannes Schindelin via GitGitGadget wrote:
>
> > Johannes Schindelin (2):
> > rebase --autostash: demonstrate a problem with dirty submodules
> > rebase --autostash: fix issue with dirty submodules
> >
> > builtin/rebase.c| 2 +-
> > t/t3420-rebase-autostash.sh | 10 ++
> > 2 files changed, 11 insertions(+), 1 deletion(-)
>
> There's another bug with rebase.autoStash in master (and next/pu) but
> not v2.19.0. I tried to bisect bit it just comes down to 5541bd5b8f
> ("rebase: default to using the builtin rebase", 2018-08-08).
>
> Credit to a co-worker of mine who wishes to remain anonymous for
> discovering this. I narrowed down his test-case to (any repo will do):
>
> (
> rm -rf /tmp/todo &&
> git clone --single-branch --no-tags --branch=todo
> https://github.com/git/git.git /tmp/todo &&
> cd /tmp/todo &&
> rm Make &&
> git rev-parse --abbrev-ref HEAD &&
> git -c rebase.autoStash=true -c pull.rebase=true pull &&
> if test $(git rev-parse --abbrev-ref HEAD) != 'todo'
> then
> echo 'On detached head!' &&
> git status &&
> exit 1
> else
> echo 'We are still on our todo branch!'
> fi
> )
I found the culprit. Patch forthcoming,
Dscho
[PATCH 0/1] http: add support selecting http version
Normally, git doesn't need to set curl to select the HTTP version, it works fine without HTTP2. Adding HTTP2 support is a icing on the cake. When http.version=20 is set, git will attempt to request the server using HTTP2. If the remote server does not support HTTP2, it is no different. Currently bitbucket supports HTTP2 and is available for testing. example: GIT_CURL_VERBOSE=1 git2 -c http.version=20 ls-remote https://bitbucket.org/aquariusjay/deeplab-public-ver2.git Force Charlie (1): http: add support selecting http version http.c | 12 1 file changed, 12 insertions(+) base-commit: 8858448bb49332d353febc078ce4a3abcc962efe Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-69%2Ffcharlie%2Fmaster-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-69/fcharlie/master-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/69 -- gitgitgadget
[PATCH 1/1] http: add support selecting http version
From: Force Charlie
Signed-off-by: Force Charlie
---
http.c | 12
1 file changed, 12 insertions(+)
diff --git a/http.c b/http.c
index 3dc8c560d6..99cb04faba 100644
--- a/http.c
+++ b/http.c
@@ -48,6 +48,7 @@ char curl_errorstr[CURL_ERROR_SIZE];
static int curl_ssl_verify = -1;
static int curl_ssl_try;
+static int curl_http_version = 11;
static const char *ssl_cert;
static const char *ssl_cipherlist;
static const char *ssl_version;
@@ -284,6 +285,10 @@ static void process_curl_messages(void)
static int http_options(const char *var, const char *value, void *cb)
{
+ if (!strcmp("http.version",var)) {
+ curl_http_version=git_config_int(var,value);
+ return 0;
+ }
if (!strcmp("http.sslverify", var)) {
curl_ssl_verify = git_config_bool(var, value);
return 0;
@@ -806,6 +811,13 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_SSL_VERIFYHOST, 2);
}
+#if LIBCURL_VERSION_NUM >= 0x073100
+ if(curl_http_version == 20){
+ /* CURL Enable HTTP2*/
+ curl_easy_setopt(result,
CURLOPT_HTTP_VERSION,CURL_HTTP_VERSION_2);
+ }
+#endif
+
#if LIBCURL_VERSION_NUM >= 0x070907
curl_easy_setopt(result, CURLOPT_NETRC, CURL_NETRC_OPTIONAL);
#endif
--
gitgitgadget
Re: [PATCH v3 1/2] range-diff doc: add a section about output stability
On Wednesday, November 7, 2018 5:22:01 AM MST Ævar Arnfjörð Bjarmason wrote: > +OUTPUT STABILITY > + > + > +The output of the `range-diff` command is subject to change. It is > +intended to be human-readable porcelain output, not something that can > +be used across versions of Git to get a textually stable `range-diff` > +(as opposed to something like the `--stable` option to > +linkgit:git-patch-id[1]). There's also no equivalent of > +linkgit:git-apply[1] for `range-diff`, the output is not intended to > +be machine-readable. > + > +This is particularly true when passing in diff options. Currently some > +options like `--stat` can as an emergent effect produce output that's "`--stat` can as an emergent": I read that for times to decided it was correct grammar. Should it be reworded to read better? Just a nit. > +quite useless in the context of `range-diff`. Future versions of > +`range-diff` may learn to interpret such options in a manner specifc > +to `range-diff` (e.g. for `--stat` summarizing how the diffstat > +changed).
Re: What's cooking in git.git (Nov 2018, #03; Wed, 7)
On Wed, Nov 07, 2018 at 06:41:45PM +0900, Junio C Hamano wrote:
> * nd/i18n (2018-11-06) 16 commits
> - fsck: mark strings for translation
> - fsck: reduce word legos to help i18n
> - parse-options.c: mark more strings for translation
> - parse-options.c: turn some die() to BUG()
> - parse-options: replace opterror() with optname()
> - repack: mark more strings for translation
> - remote.c: mark messages for translation
> - remote.c: turn some error() or die() to BUG()
> - reflog: mark strings for translation
> - read-cache.c: add missing colon separators
> - read-cache.c: mark more strings for translation
> - read-cache.c: turn die("internal error") to BUG()
> - attr.c: mark more string for translation
> - archive.c: mark more strings for translation
> - alias.c: mark split_cmdline_strerror() strings for translation
> - git.c: mark more strings for translation
>
> More _("i18n") markings.
When this patch is merged into 'pu' all four tests added to
't1450-fsck.sh' in b29759d89a (fsck: check HEAD and reflog from other
worktrees, 2018-10-21) as part of 'nd/per-worktree-ref-iteration'
below fail when run with GETTEXT_POISON=y. The test suite passes in
both of these topics on their own, even with GETTEXT_POISON, it's
their merge that is somehow problematic.
> * nd/per-worktree-ref-iteration (2018-11-05) 9 commits
> (merged to 'next' on 2018-11-06 at 53803cedf3)
> + git-worktree.txt: correct linkgit command name
> (merged to 'next' on 2018-11-03 at 4cbe49a704)
> + reflog expire: cover reflog from all worktrees
> + fsck: check HEAD and reflog from other worktrees
> + fsck: move fsck_head_link() to get_default_heads() to avoid some globals
> + revision.c: better error reporting on ref from different worktrees
> + revision.c: correct a parameter name
> + refs: new ref types to make per-worktree refs visible to all worktrees
> + Add a place for (not) sharing stuff between worktrees
> + refs.c: indent with tabs, not spaces
>
> The code to traverse objects for reachability, used to decide what
> objects are unreferenced and expendable, have been taught to also
> consider per-worktree refs of other worktrees as starting points to
> prevent data loss.
>
> Will merge to 'master'.
Re: Wildcard URL config matching
Hi Brian,
On Mon, Nov 05, 2018 at 08:56:52PM +, brian m. carlson wrote:
> In a272b9e70a ("urlmatch: allow globbing for the URL host part",
> 2017-01-31), we added support for wildcard matching for URLs when
> reading from .git/config. Now it's possible to specify an option like
> http.http://*.example.com/.cookieFile and have it match for the URL
> http://foo.example.com. However, since this option also allows
> wildcards at any level, the following also matches:
> http.http://*.*.*/.cookieFile.
>
> I'm wondering if it was intentional to allow this behavior or if we
> intended to allow only the leftmost label (or labels) to be a wildcard.
> The tests seem to test only the leftmost label, and that is the behavior
> that one has for TLS certificates, for example. I don't really see a
> situation where one would want to match hostname labels in an arbitrary
> position, but perhaps I'm simply not being imaginative enough in
> thinking through the use cases.
The behavior is intentional. The above example of "http://*.*.*/;
obviously doesn't make a lot of sense, but e.g. the pattern
"http://*.*.example.com; should match all sub-sub domains of
"example.com" like for example "http://foo.bar.example.com;. The
ability to use multiple globs is necessary as they aren't "true"
globs, but will only match the current component of the subdomain
separated by dots. So they behave similar to globs in the shell,
where they only match up to the next directory separator.
If we were to remove the ability to use globs for multiple
components, then one would have to again explicitly list patterns
for all possible combinations.
> Regardless of what we decide, I'll be sending a patch, either to add
> additional tests, or correct the code (or both).
I agree that there should've been additional tests to also verify
that multiple globs work as expected. So thanks for doing that!
> I ask because we're implementing this behavior for Git LFS, where we
> don't iterate over all configuration keys, instead looking up certain
> values in a hash. We'll need to make some changes in order to have
> things work correctly if we want to implement the current Git behavior
> to prevent combinatorial explosion.
Regards
Patrick
signature.asc
Description: PGP signature
Re: What's cooking in git.git (Nov 2018, #03; Wed, 7)
Ævar Arnfjörð Bjarmason writes: > On Wed, Nov 07 2018, Junio C Hamano wrote: > >> * ab/range-diff-no-patch (2018-11-07) 1 commit >> - range-diff: add a --no-patch option to show a summary >> >> "range-diff" learns the "--no-patch" option, which can be used to >> get a high-level overview without the actual line-by-line patch >> difference shown. >> >> Will merge to 'next'. > > Per <20181107122202.1813-1-ava...@gmail.com> it turns out this whole > thing should have been a bugfix instead, sent a v2. Thanks.
Re: What's cooking in git.git (Nov 2018, #03; Wed, 7)
On Wed, Nov 07 2018, Junio C Hamano wrote: > * ab/range-diff-no-patch (2018-11-07) 1 commit > - range-diff: add a --no-patch option to show a summary > > "range-diff" learns the "--no-patch" option, which can be used to > get a high-level overview without the actual line-by-line patch > difference shown. > > Will merge to 'next'. Per <20181107122202.1813-1-ava...@gmail.com> it turns out this whole thing should have been a bugfix instead, sent a v2. > * ab/dynamic-gettext-poison (2018-11-02) 1 commit > - i18n: make GETTEXT_POISON a runtime option > > On hold. > cf. <20181102163725.gy30...@szeder.dev> Hope to sent update soon... > * ab/push-dwim-dst (2018-10-29) 9 commits > - SQUASH??? > - push doc: document the DWYM behavior pushing to unqualified > - push: add DWYM support for "git push refs/remotes/...:" > - push: test that doesn't DWYM if is unqualified > - push: add an advice on unqualified push > - push: move unqualified refname error into a function > - push: improve the error shown on unqualified push > - i18n: remote.c: mark error(...) messages for translation > - remote.c: add braces in anticipation of a follow-up change > > "git push $there $src:$dst" rejects when $dst is not a fully > qualified refname and not clear what the end user meant. The > codepath has been taught to give a clearer error message, and also > guess where the push should go by taking the type of the pushed > object into account (e.g. a tag object would want to go under > refs/tags/). > > The last few steps are questionable. > cf. <87in1lkw54@evledraar.gmail.com> ...ditto... > * ab/pack-tests-cleanup (2018-10-31) 3 commits > (merged to 'next' on 2018-11-03 at b4a39595bb) > + index-pack tests: don't leave test repo dirty at end > + pack-objects tests: don't leave test .git corrupt at end > + pack-objects test: modernize style > > A couple of tests used to leave the repository in a state that is > deliberately corrupt, which have been corrected. > > Will merge to 'master'. Thanks! > * nd/config-split (2018-10-29) 79 commits > (merged to 'next' on 2018-11-03 at a336559101) > + config.txt: remove config/dummy.txt > + config.txt: move worktree.* to a separate file > + config.txt: move web.* to a separate file > + config.txt: move versionsort.* to a separate file > + config.txt: move user.* to a separate file > + config.txt: move url.* to a separate file > + config.txt: move uploadpack.* to a separate file > + config.txt: move uploadarchive.* to a separate file > + config.txt: move transfer.* to a separate file > + config.txt: move tag.* to a separate file > + config.txt: move submodule.* to a separate file > + config.txt: move stash.* to a separate file > + config.txt: move status.* to a separate file > + config.txt: move splitIndex.* to a separate file > + config.txt: move showBranch.* to a separate file > + config.txt: move sequencer.* to a separate file > + config.txt: move sendemail-config.txt to config/ > + config.txt: move reset.* to a separate file > + config.txt: move rerere.* to a separate file > + config.txt: move repack.* to a separate file > + config.txt: move remotes.* to a separate file > + config.txt: move remote.* to a separate file > + config.txt: move receive-config.txt to config/ > + config.txt: move rebase-config.txt to config/ > + config.txt: move push-config.txt to config/ > + config.txt: move pull-config.txt to config/ > + config.txt: move protocol.* to a separate file > + config.txt: move pretty.* to a separate file > + config.txt: move pager.* to a separate file > + config.txt: move pack.* to a separate file > + config.txt: move notes.* to a separate file > + config.txt: move mergetool.* to a separate file > + config.txt: move merge-config.txt to config/ > + config.txt: move man.* to a separate file > + config.txt: move mailmap.* to a separate file > + config.txt: move mailinfo.* to a separate file > + config.txt: move log.* to a separate file > + config.txt: move interactive.* to a separate file > + config.txt: move instaweb.* to a separate file > + config.txt: move init.* to a separate file > + config.txt: move index.* to a separate file > + git-imap-send.txt: move imap.* to a separate file > + config.txt: move i18n.* to a separate file > + config.txt: move http.* to a separate file > + config.txt: move ssh.* to a separate file > + config.txt: move help.* to a separate file > + config.txt: move guitool.* to a separate file > + config.txt: move gui-config.txt to config/ > + config.txt: move gpg.* to a separate file > + config.txt: move grep.* to a separate file > + config.txt: move gitweb.* to a separate file > + config.txt: move gitcvs-config.txt to config/ > + config.txt: move gc.* to a separate file > + config.txt: move fsck.* to a separate file > + config.txt: move fmt-merge-msg-config.txt to config/ > + config.txt: move format-config.txt to config/ > + config.txt: move filter.* to a
[PATCH v3 0/2] range-diff: doc + regression fix
As Johannes notes this --no-patch option I wanted to add is something we had already, but is it turns out it was broken. So this is an entirely rewritten v3 (not bothering with the range-diff for it) which a) documents the output stability of stuff like --stat and the like (there isn't any) b) fixes the regression & adds a test. I did try various other diff options and they all seem to work. Ævar Arnfjörð Bjarmason (2): range-diff doc: add a section about output stability range-diff: fix regression in passing along diff options Documentation/git-range-diff.txt | 17 + range-diff.c | 3 ++- t/t3206-range-diff.sh| 30 ++ 3 files changed, 49 insertions(+), 1 deletion(-) -- 2.19.1.930.g4563a0d9d0
[PATCH v3 1/2] range-diff doc: add a section about output stability
The range-diff command is already advertised as porcelain, but let's make it really clear that the output is completely subject to change, particularly when it comes to diff options such as --stat. Right now that option doesn't work, but fixing that is the subject of a later change. Signed-off-by: Ævar Arnfjörð Bjarmason --- Documentation/git-range-diff.txt | 17 + 1 file changed, 17 insertions(+) diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt index f693930fdb..bbd07a9be8 100644 --- a/Documentation/git-range-diff.txt +++ b/Documentation/git-range-diff.txt @@ -78,6 +78,23 @@ between patches", i.e. to compare the author, commit message and diff of corresponding old/new commits. There is currently no means to tweak the diff options passed to `git log` when generating those patches. +OUTPUT STABILITY + + +The output of the `range-diff` command is subject to change. It is +intended to be human-readable porcelain output, not something that can +be used across versions of Git to get a textually stable `range-diff` +(as opposed to something like the `--stable` option to +linkgit:git-patch-id[1]). There's also no equivalent of +linkgit:git-apply[1] for `range-diff`, the output is not intended to +be machine-readable. + +This is particularly true when passing in diff options. Currently some +options like `--stat` can as an emergent effect produce output that's +quite useless in the context of `range-diff`. Future versions of +`range-diff` may learn to interpret such options in a manner specifc +to `range-diff` (e.g. for `--stat` summarizing how the diffstat +changed). CONFIGURATION - -- 2.19.1.930.g4563a0d9d0
[PATCH v3 2/2] range-diff: fix regression in passing along diff options
In 73a834e9e2 ("range-diff: relieve callers of low-level configuration
burden", 2018-07-22) we broke passing down options like --no-patch,
--stat etc. Fix that regression, and add a test for some of these
options being passed down.
As noted in a change leading up to this ("range-diff doc: add a
section about output stability", 2018-11-07) the output is not meant
to be stable. So this regression test will likely need to be tweaked
once we get a "proper" --stat option.
See
https://public-inbox.org/git/nycvar.qro.7.76.6.1811071202480...@tvgsbejvaqbjf.bet/
for a further explanation of the regression.
The quoting of "EOF" here mirrors that of an earlier test. Perhaps
that should be fixed, but let's leave that up to a later cleanup
change.
Signed-off-by: Ævar Arnfjörð Bjarmason
---
range-diff.c | 3 ++-
t/t3206-range-diff.sh | 30 ++
2 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/range-diff.c b/range-diff.c
index bd8083f2d1..488844c0af 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -453,7 +453,8 @@ int show_range_diff(const char *range1, const char *range2,
struct strbuf indent = STRBUF_INIT;
memcpy(, diffopt, sizeof(opts));
- opts.output_format = DIFF_FORMAT_PATCH;
+ if (!opts.output_format)
+ opts.output_format = DIFF_FORMAT_PATCH;
opts.flags.suppress_diff_headers = 1;
opts.flags.dual_color_diffed_diffs = dual_color;
opts.output_prefix = output_prefix_cb;
diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index 6aae364171..e497c1358f 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh
@@ -122,6 +122,36 @@ test_expect_success 'changed commit' '
test_cmp expected actual
'
+test_expect_success 'changed commit with --no-patch diff option' '
+ git range-diff --no-color --no-patch topic...changed >actual &&
+ cat >expected <<-EOF &&
+ 1: 4de457d = 1: a4b s/5/A/
+ 2: fccce22 = 2: f51d370 s/4/A/
+ 3: 147e64e ! 3: 0559556 s/11/B/
+ 4: a63e992 ! 4: d966c5c s/12/B/
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'changed commit with --stat diff option' '
+ git range-diff --no-color --stat topic...changed >actual &&
+ cat >expected <<-EOF &&
+ 1: 4de457d = 1: a4b s/5/A/
+a => b | 0
+1 file changed, 0 insertions(+), 0 deletions(-)
+ 2: fccce22 = 2: f51d370 s/4/A/
+a => b | 0
+1 file changed, 0 insertions(+), 0 deletions(-)
+ 3: 147e64e ! 3: 0559556 s/11/B/
+a => b | 0
+1 file changed, 0 insertions(+), 0 deletions(-)
+ 4: a63e992 ! 4: d966c5c s/12/B/
+a => b | 0
+1 file changed, 0 insertions(+), 0 deletions(-)
+ EOF
+ test_cmp expected actual
+'
+
test_expect_success 'changed commit with sm config' '
git range-diff --no-color --submodule=log topic...changed >actual &&
cat >expected <<-EOF &&
--
2.19.1.930.g4563a0d9d0
Re: build error on mac os 10.14.1
Solved, It is because the macOS SDK headers loss in 10.14.1. Need install command line tool 10.1 and install the header package in /Library/Developer/CommandLineTools/Packages/macOS_SDK_headers_for_macOS_10.14.pkg So I think the git make configure need check the SDK header to get the correct reason of build fail yan ke 于2018年11月6日周二 下午5:07写道： > > Hello > > when build on mac os 10.14.1 with the master branch, I got the > error as blew, so what is wrong? > > ld: archive has no table of contents file 'xdiff/lib.a' for architecture > x86_64 > ld: archive has no table of contents file 'xdiff/lib.a' for architecture > x86_64 > ld: archive has no table of contents file 'xdiff/lib.a' for architecture > x86_64 > ld: archive has no table of contents file 'xdiff/lib.a' for architecture > x86_64 > ld: archive has no table of contents file 'xdiff/lib.a' for architecture > x86_64 > ld: archive has no table of contents file 'xdiff/lib.a' for architecture > x86_64 > clangclang: error: linker command failed with exit code 1 (use -v to > see invocation) > : error: linker command failed with exit code 1 (use -v to see invocation) > clang: error: linker command failed with exit code 1 (use -v to see > invocation) > clangmake: *** [Makefile:2369: git-shell] Error 1 > make: *** Waiting for unfinished jobs > make: *** [Makefile:2369: git-sh-i18n--envsubst] Error 1 > : error: linker command failed with exit code 1 (use -v to see invocation) > clang: make: *** [Makefile:2369: git-credential-cache--daemon] Error 1 > error: linker command failed with exit code 1 (use -v to see invocation) > clang: error: linker command failed with exit code 1 (use -v to see > invocation) > make: *** [Makefile:2369: git-credential-cache] Error 1 > make: *** [Makefile:2369: git-credential-store] Error 1 > make: *** [Makefile:2383: git-remote-testsvn] Error 1 > ld: archive has no table of contents file 'xdiff/lib.a' for architecture > x86_64 > clang: error: linker command failed with exit code 1 (use -v to see > invocation) > make: *** [Makefile:2393: git-remote-http] Error 1 > ld: archive has no table of contents file 'xdiff/lib.a' for architecture > x86_64 > ld: archive has no table of contents file 'xdiff/lib.a' for architecture > x86_64 > ld: archive has no table of contents file 'xdiff/lib.a' for architecture > x86_64 > ld: archive has no table of contents file 'xdiff/lib.a' for architecture > x86_64 > clang: error: linker command failed with exit code 1 (use -v to see > invocation) > ld: archive has no table of contents file 'xdiff/lib.a' for architecture > x86_64 > clang: error: linker command failed with exit code 1 (use -v to see > invocation) > make: *** [Makefile:2369: git-http-backend] Error 1 > clang: error: linker command failed with exit code 1 (use -v to see > invocation) > make: *** [Makefile:2372: git-imap-send] Error 1 > make: *** [Makefile:2379: git-http-push] Error 1 > clang: error: linker command failed with exit code 1 (use -v to see > invocation) > clang: error: linker command failed with exit code 1 (use -v to see > invocation) > make: *** [Makefile:2376: git-http-fetch] Error 1 > make: *** [Makefile:2369: git-daemon] Error 1 > ld: archive has no table of contents file 'xdiff/lib.a' for architecture > x86_64 > clang: error: linker command failed with exit code 1 (use -v to see > invocation) > make: *** [Makefile:2369: git-fast-import] Error 1 > ld: archive has no table of contents file 'xdiff/lib.a' for architecture > x86_64 > clang: error: linker command failed with exit code 1 (use -v to see > invocation) > make: *** [Makefile:2046: git] Error 1
Re: [PATCH 1/1] Windows: force-recompile git.res for differing architectures
Hi, On Wed, 7 Nov 2018, Junio C Hamano wrote: > Johannes Sixt writes: > > > Am 07.11.18 um 02:32 schrieb Junio C Hamano: > >> Johannes Sixt writes: > >>> On Linux, when I recompile for a different architecture, CFLAGS would > >>> change, so I would have thought that GIT-CFLAGS were the natural > >>> choice for a dependency. Don't they change in this case on Windows, > >>> too? > >> > >> Depending on GIT-CFLAGS would have a better chance of being safe, I > >> guess, even though it can at times be overly safe, than GIT-PREFIX, > >> I suspect. As a single user target in Makefile, which is only used > >> by Dscho, who intends to stick to /mingw(32|64)/ convention til the > >> end of time, I think the posted patch is OK, though. > > > > I think that it's not only Dscho who uses the target (my build > > environment, for example, is different from Dscho's and compiles > > git.res, too). But since the patch helps him most and doesn't hurt > > others, it is good to go. No objection from my side. > > Yeah, that was phrased poorly. What I meant was by "only by Dscho" > was "only by those who share the convention that GIT-PREFIX is > changed if and only if targetting a different arch". > > Anyway, I just wanted to say that GIT-PREFIX may not be precise > enough but would give sufficient signal; GIT-CFLAGS may be a more > cautious choice, but it may change a bit too often ;-). I am fine with GIT-CFLAGS, too. Do you want me to "rick-roll" a v2? Ciao, Dscho
Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()
Hi Hannes,
On Tue, 6 Nov 2018, Johannes Sixt wrote:
> Am 06.11.18 um 15:53 schrieb Johannes Schindelin via GitGitGadget:
> > From: Johannes Schindelin
> >
> > On Windows, an absolute POSIX path needs to be turned into a Windows
> > one.
>
> If I were picky, I would say that in a pure Windows application there cannot
> be POSIX paths to begin with.
>
> Even if a path looks like a POSIX paths, i.e. it starts with a directory
> separator, but not with drive-letter-colon, it still has a particular meaning,
> namely (as far as I know) that the path is anchored at the root of the drive
> of the current working directory.
>
> If a user specifies such a path on Windows, and it must undergo
> expand_user_path(), then that is a user error, or the user knows what they are
> doing.
>
> I think it is wrong to interpret such a path as relative to some random other
> directory, like this patch seems to do.
Okay, now we know everything you find wrong with the current patch. Do you
have any suggestion how to make it right? I.e. what would you suggest as a
way to specify in a gitconfig in a portable Git where the certificate
bundle is?
Thanks,
Dscho
>
> >
> > Signed-off-by: Johannes Schindelin
> > ---
> > path.c | 5 +
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/path.c b/path.c
> > index 34f0f98349..a72abf0e1f 100644
> > --- a/path.c
> > +++ b/path.c
> > @@ -11,6 +11,7 @@
> > #include "path.h"
> > #include "packfile.h"
> > #include "object-store.h"
> > +#include "exec-cmd.h"
> >
> > static int get_st_mode_bits(const char *path, int *mode)
> > {
> > @@ -709,6 +710,10 @@ char *expand_user_path(const char *path, int real_home)
> >
> >if (path == NULL)
> > goto return_null;
> > +#ifdef __MINGW32__
> > + if (path[0] == '/')
> > + return system_path(path + 1);
> > +#endif
> >if (path[0] == '~') {
> > const char *first_slash = strchrnul(path, '/');
> > const char *username = path + 1;
> >
>
>
>
Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()
Hi,
On Tue, 6 Nov 2018, Duy Nguyen wrote:
> On Tue, Nov 6, 2018 at 3:55 PM Johannes Schindelin via GitGitGadget
> wrote:
> >
> > From: Johannes Schindelin
> >
> > On Windows, an absolute POSIX path needs to be turned into a Windows
> > one.
> >
> > Signed-off-by: Johannes Schindelin
> > ---
> > path.c | 5 +
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/path.c b/path.c
> > index 34f0f98349..a72abf0e1f 100644
> > --- a/path.c
> > +++ b/path.c
> > @@ -11,6 +11,7 @@
> > #include "path.h"
> > #include "packfile.h"
> > #include "object-store.h"
> > +#include "exec-cmd.h"
> >
> > static int get_st_mode_bits(const char *path, int *mode)
> > {
> > @@ -709,6 +710,10 @@ char *expand_user_path(const char *path, int real_home)
> >
> > if (path == NULL)
> > goto return_null;
> > +#ifdef __MINGW32__
> > + if (path[0] == '/')
> > + return system_path(path + 1);
> > +#endif
>
> Should this behavior be documented somewhere, maybe in config.txt?
First we need to find a consensus how this should work.
Ciao,
Dscho
>
> > if (path[0] == '~') {
> > const char *first_slash = strchrnul(path, '/');
> > const char *username = path + 1;
> > --
> > gitgitgadget
> --
> Duy
>
Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()
Hi,
On Wed, 7 Nov 2018, Junio C Hamano wrote:
> Ramsay Jones writes:
>
> > On 06/11/2018 14:53, Johannes Schindelin via GitGitGadget wrote:
> >> From: Johannes Schindelin
> >>
> >> On Windows, an absolute POSIX path needs to be turned into a Windows
> >> one.
> >>
> >> Signed-off-by: Johannes Schindelin
> >> ---
> >> path.c | 5 +
> >> 1 file changed, 5 insertions(+)
> >>
> >> diff --git a/path.c b/path.c
> >> index 34f0f98349..a72abf0e1f 100644
> >> --- a/path.c
> >> +++ b/path.c
> >> @@ -11,6 +11,7 @@
> >> #include "path.h"
> >> #include "packfile.h"
> >> #include "object-store.h"
> >> +#include "exec-cmd.h"
> >>
> >> static int get_st_mode_bits(const char *path, int *mode)
> >> {
> >> @@ -709,6 +710,10 @@ char *expand_user_path(const char *path, int
> >> real_home)
> >>
> >>if (path == NULL)
> >>goto return_null;
> >> +#ifdef __MINGW32__
> >> + if (path[0] == '/')
> >> + return system_path(path + 1);
> >> +#endif
> >
> > Hmm, this doesn't quite fit with the intended use of this
> > function! ;-) (even on windows!)
> >
> > I haven't looked very deeply, but doesn't this affect all
> > absolute paths in the config read by git_config_pathname(),
> > along with all 'included config' files?
>
> I think so. I have not thought things through to say if replacing a
> "full path in the current drive" with system_path() is a sensible
> thing to do in the first place, but I am getting the impression from
> review comments that it probably is not.
>
> > I am pretty sure that I would not want the absolute paths
> > in my config file(s) magically 'moved' depending on whether
> > git has been compiled with 'runtime prefix' support or not!
The cute thing is: your absolute paths would not be moved because we are
talking about Windows. Therefore your absolute paths would not start with
a forward slash.
> In any case, the helper is about expanding ~/foo and ~who/foo to
> absolute paths, without touching other paths, so it is a wrong
> function to implement it in, even if the motivation were sensible.
It could be renamed. In any case, for this feature we would need to expand
a path that is not the final path, and this here location is the most
logical place to do so.
Ciao,
Dscho
Re: [PATCH v2] range-diff: add a --no-patch option to show a summary
Hi,
On Wed, 7 Nov 2018, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason writes:
>
> > diff --git a/builtin/range-diff.c b/builtin/range-diff.c
> > index f01a0be851..05d1f6b6b6 100644
> > --- a/builtin/range-diff.c
> > +++ b/builtin/range-diff.c
> > @@ -16,11 +16,14 @@ int cmd_range_diff(int argc, const char **argv, const
> > char *prefix)
> > int creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;
> > struct diff_options diffopt = { NULL };
> > int simple_color = -1;
> > + int no_patch = 0;
> > struct option options[] = {
> > OPT_INTEGER(0, "creation-factor", _factor,
> > N_("Percentage by which creation is weighted")),
> > OPT_BOOL(0, "no-dual-color", _color,
> > N_("use simple diff colors")),
> > + OPT_BOOL_F('s', "no-patch", _patch,
> > +N_("show patch output"), PARSE_OPT_NONEG),
>
> As OPT_BOOL("patch") natively takes "--no-patch" to flip the bool
> off, an int variable "patch" that is initialized to 1 would make it
> more readable by avoiding double negation !no_patch like the one we
> see below. I guess the reason behind the contortion you wanted to
> give the synonym --silent to it?
In light of my investigation that revealed that the original behavior
(which is still documented in the manual page of range-diff) was broken,
and I would much rather see that fixed than adding a workaround.
I would be fine with my patch being combined with the update to the manual
page and the regression test, as a v3.
Ciao,
Dscho
>
> > @@ -92,7 +95,7 @@ int cmd_range_diff(int argc, const char **argv, const
> > char *prefix)
> > }
> >
> > res = show_range_diff(range1.buf, range2.buf, creation_factor,
> > - simple_color < 1, );
> > + simple_color < 1, !no_patch, );
>
> > strbuf_release();
> > strbuf_release();
>
> > @@ -7,6 +7,7 @@
> >
> > int show_range_diff(const char *range1, const char *range2,
> > int creation_factor, int dual_color,
> > + int patch,
> > struct diff_options *diffopt);
>
> Other than that small "Huh?", the code looks good to me.
>
> > diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
> > index 6aae364171..27e071650b 100755
> > --- a/t/t3206-range-diff.sh
> > +++ b/t/t3206-range-diff.sh
> > @@ -122,6 +122,26 @@ test_expect_success 'changed commit' '
> > test_cmp expected actual
> > '
> >
> > +test_expect_success 'changed commit -p & --patch' '
> > + git range-diff --no-color -p topic...changed >actual &&
> > + test_cmp expected actual &&
> > + git range-diff --no-color --patch topic...changed >actual &&
> > + test_cmp expected actual
>
> This makes sure that -p and --patch produces the same output as the
> default case? I am not sure who in the parseopt API groks the
> single letter "-p" in this case offhand. Care to explain how?
>
> The other side of the test to see -s/--no-patch we see below also
> makes sense.
>
> > +test_expect_success 'changed commit -s & --no-patch' '
> > +...
> > + cat >expected <<-EOF &&
>
> Quote EOF to allow readers skim the contents without looking for and
> worrying about $substitutions in there, unless there are tons of
> offending code in the same script already in which case we should
> leave the clean-up outside this primary change.
>
>
Re: [PATCH] range-diff: add a --no-patch option to show a summary
Me again, On Wed, 7 Nov 2018, Johannes Schindelin wrote: > On Wed, 7 Nov 2018, Johannes Schindelin wrote: > > > On Wed, 7 Nov 2018, Johannes Schindelin wrote: > > > > > On Tue, 6 Nov 2018, Ævar Arnfjörð Bjarmason wrote: > > > > > > > On Tue, Nov 06 2018, Johannes Schindelin wrote: > > > > > > > > > On Mon, 5 Nov 2018, Ævar Arnfjörð Bjarmason wrote: > > > > > > > > > >> On Mon, Nov 05 2018, Eric Sunshine wrote: > > > > >> > > > > >> > On Mon, Nov 5, 2018 at 3:07 PM Ævar Arnfjörð Bjarmason > > > > >> > wrote: > > > > >> >> Add a --no-patch option which shows which changes got removed, > > > > >> >> added > > > > >> >> or moved etc., without showing the diff associated with them. > > > > >> > > > > > >> > This option existed in the very first version[1] of range-diff > > > > >> > (then > > > > >> > called branch-diff) implemented by Dscho, although it was called > > > > >> > --no-patches (with an "es"), which it inherited from tbdiff. I > > > > >> > think > > > > >> > someone (possibly me) pointed out that --no-patch (sans "es") > > > > >> > would be > > > > >> > more consistent with existing Git options. I don't recall why Dscho > > > > >> > removed the option during the re-rolls, but the explanation may be > > > > >> > in > > > > >> > that thread. > > > > >> > > > > >> Thanks for digging. Big thread, not going to re-read it now. I'd just > > > > >> like to have this. > > > > > > > > > > In my hands, the well-documented `-s` option works (see e.g. > > > > > https://git-scm.com/docs/git-diff#git-diff--s), although I have to > > > > > admit > > > > > that the `git-range-diff` manual does not talk about the diff-options. > > > > > > > > > > And for the record, for me, `git range-diff A...B --no-patch` > > > > > *already* > > > > > works. > > > > > > > > Neither of those works for me without my patch. E.g. > > > > > > > > ./git-range-diff -s 711aaa392f...a5ba8f2101 > > > > ./git-range-diff --no-patch 711aaa392f...a5ba8f2101 > > > > > > > > This is on current next, 2.19.1.1182.g4ecb1133ce. What version are you > > > > on? > > > > > > I tried it with git version 2.19.0.windows.1. > > > > > > To verify, I repeated this with `next` (git version > > > 2.19.1.1215.g8438c0b2453a): > > > > > > ./git range-diff -s 711aaa392f...a5ba8f2101 > > > fatal: unrecognized argument: --output-indicator-new=> > > > error: could not parse log for 'a5ba8f2101..711aaa392f' > > > > > > Which means that something broke rather dramatically between > > > v2.19.0.windows.1 and 8438c0b2453a. > > > > Nevermind, this was solved by passing `--exec-path=$PWD`. And *now* I can > > reproduce your finding. > > I've bisected this breakage down to 73a834e9e279 (range-diff: relieve > callers of low-level configuration burden, 2018-07-22). Eric, that's one > of your commits. Okay, so I would really like to point you to this particular paragraph in the manual page of `git range-diff` (just below https://git-scm.com/docs/git-range-diff#git-range-diff-ltbasegtltrev1gtltrev2gt): `git range-diff` also accepts the regular diff options (see linkgit:git-diff[1]), most notably the `--color=[]` and `--no-color` options. These options are used when generating the "diff between patches", i.e. to compare the author, commit message and diff of corresponding old/new commits. There is currently no means to tweak the diff options passed to `git log` when generating those patches. So this was quite intentional, in particular with an eye on `--no-patch`. Note also the commit message of c8c5e43ac3f9 (range-diff: also show the diff between patches, 2018-08-13): Note also: while tbdiff accepts the `--no-patches` option to suppress these diffs between patches, we prefer the `-s` (or `--no-patch`) option that is automatically supported via our use of diff_opt_parse(). This was very, very intentional, as you can see, and it was pretty broken by 73a834e. This here patch papers over that breakage, sadly I have too much on my plate as it is, so I cannot wrap it up in a nice commit (nor add a regression test, but you did that, nor investigate what else is broken) and therefore I would be indebted if you could take this in your custody: diff --git a/range-diff.c b/range-diff.c index 3dd2edda0176..014112ee401f 100644 --- a/range-diff.c +++ b/range-diff.c @@ -433,7 +433,8 @@ int show_range_diff(const char *range1, const char *range2, struct strbuf indent = STRBUF_INIT; memcpy(, diffopt, sizeof(opts)); - opts.output_format = DIFF_FORMAT_PATCH; + if (!opts.output_format) + opts.output_format = DIFF_FORMAT_PATCH; opts.flags.suppress_diff_headers = 1; opts.flags.dual_color_diffed_diffs = dual_color; opts.output_prefix = output_prefix_cb; Ciao, Dscho
Re: [PATCH] range-diff: add a --no-patch option to show a summary
Hi Ævar, On Wed, 7 Nov 2018, Johannes Schindelin wrote: > On Wed, 7 Nov 2018, Johannes Schindelin wrote: > > > On Tue, 6 Nov 2018, Ævar Arnfjörð Bjarmason wrote: > > > > > On Tue, Nov 06 2018, Johannes Schindelin wrote: > > > > > > > On Mon, 5 Nov 2018, Ævar Arnfjörð Bjarmason wrote: > > > > > > > >> On Mon, Nov 05 2018, Eric Sunshine wrote: > > > >> > > > >> > On Mon, Nov 5, 2018 at 3:07 PM Ævar Arnfjörð Bjarmason > > > >> > wrote: > > > >> >> Add a --no-patch option which shows which changes got removed, added > > > >> >> or moved etc., without showing the diff associated with them. > > > >> > > > > >> > This option existed in the very first version[1] of range-diff (then > > > >> > called branch-diff) implemented by Dscho, although it was called > > > >> > --no-patches (with an "es"), which it inherited from tbdiff. I think > > > >> > someone (possibly me) pointed out that --no-patch (sans "es") would > > > >> > be > > > >> > more consistent with existing Git options. I don't recall why Dscho > > > >> > removed the option during the re-rolls, but the explanation may be in > > > >> > that thread. > > > >> > > > >> Thanks for digging. Big thread, not going to re-read it now. I'd just > > > >> like to have this. > > > > > > > > In my hands, the well-documented `-s` option works (see e.g. > > > > https://git-scm.com/docs/git-diff#git-diff--s), although I have to admit > > > > that the `git-range-diff` manual does not talk about the diff-options. > > > > > > > > And for the record, for me, `git range-diff A...B --no-patch` *already* > > > > works. > > > > > > Neither of those works for me without my patch. E.g. > > > > > > ./git-range-diff -s 711aaa392f...a5ba8f2101 > > > ./git-range-diff --no-patch 711aaa392f...a5ba8f2101 > > > > > > This is on current next, 2.19.1.1182.g4ecb1133ce. What version are you > > > on? > > > > I tried it with git version 2.19.0.windows.1. > > > > To verify, I repeated this with `next` (git version > > 2.19.1.1215.g8438c0b2453a): > > > > ./git range-diff -s 711aaa392f...a5ba8f2101 > > fatal: unrecognized argument: --output-indicator-new=> > > error: could not parse log for 'a5ba8f2101..711aaa392f' > > > > Which means that something broke rather dramatically between > > v2.19.0.windows.1 and 8438c0b2453a. > > Nevermind, this was solved by passing `--exec-path=$PWD`. And *now* I can > reproduce your finding. I've bisected this breakage down to 73a834e9e279 (range-diff: relieve callers of low-level configuration burden, 2018-07-22). Eric, that's one of your commits. Ciao, Dscho
[Market Info] Now is your chance to exhibit in Japan!
Dear International Sales & Marketing Director Zhejiang Wuchuan Industrial Co., Ltd, Hello, this is Eiichi Hasegawa from LIFESTYLE EXPO TOKYO 2019 Show Management. Today, I would like to share the information of the growing Japanese gift market, and the buyers that you can meet at our show. Please take a look at the market information below, and consider exhibiting at LIFESTYLE EXPO TOKYO 2019 [January]! - < NOW is your time to tackle Japan > - 1) Strong growth in the Japanese Gift Market. US$ 87 billion → US$ 93 billion That is over 6.8% growth rate! (*1) 2) Strong demand for Imported Gift Products in Japan. US$ 20 billion → US$ 21 billion That is a 3.4% growth rate, which is approximately more than 2 billion US dollars! (*1) - < Meet the visitors! > - At LIFESTYLE EXPO TOKYO, you can meet: 1) Importers/Distributors -- ANA TRADING, ITOCHU, MARUBENI INTEX, MITSUBISHI CORP. FASHION, etc. 2) Buyers -- Gift/Stationery Shops: ITO-YA, LOFT, MUJI, PLAZA, TOKYU HANDS, etc. -- Interior Shops/Design Stores: MOMA DESIGN STORE, FRANCFRANC, ACTUS, KEYUCA, etc. -- Concept Shops/Apparel Shops: BEAMS, FREAK’S STORE, SHIPS, URBAN RESEARCH, etc. -- Department Stores: DAIMARU MATSUZAKAYA, ISETAN MITSUKOSHI, TAKASHIMAYA, etc. 3) Key contacts for OEM or Contract Manufacturing -- Major Japanese brands, Mass retailers, GMS, etc. and more! * visitors are excerpts from the 2018 July show - For more information, please kindly fill in the REPLY FORM below. We will be happy to get back to you for details. We look forward to your reply. == Reply Form mailto:lifestyle-...@reedexpo.co.jp Company Name: Contact Person: Email: TEL: Main Products: Your Request ( ) Exhibiting Cost ( sqm) ( ) Available Booth Locations ( ) Other ( ) === Please forward this message to the person responsible for marketing/export if needed. Best regards, Eiichi Hasegawa (Mr.), Chisato Miyawaki (Ms.), Mikako Shimada (Ms.) Qu Jun (Mr.), Choi Ilyong (Mr.) LIFESTYLE EXPO TOKYO Show Management Reed Exhibitions Japan Ltd. TEL: +81-3-3349-8505 mailto:lifestyle-...@reedexpo.co.jp --- LIFESTYLE EXPO TOKYO 2019 [January] Jan. 30 (Wed.) - Feb. 1 (Fri.), 2019, Makuhari Messe, Japan https://www.lifestyle-expo-spring.jp/en/ --- (*1: Figures from Yano Research Institute Ltd.) ID:E36-G1402-0075 This message is delivered to you to provide details of exhibitions and conferences organised, co-organised, or managed by Reed Exhibitions Japan Ltd. If you would like to change your contact information, or prefer not to receive further information on this exhibition/conference, please follow the directions below. Please click the URL below and follow the directions on the website to update your e-mail and other information. https://contact.reedexpo.co.jp/expo/REED/?lg=en=ch=CHANGE Please reply to this mail changing the subject to "REMOVE FROM LIST". You will not receive any further information on this exhibition/conference.
Re: [PATCH] range-diff: add a --no-patch option to show a summary
Hi Ævar, On Wed, 7 Nov 2018, Johannes Schindelin wrote: > On Tue, 6 Nov 2018, Ævar Arnfjörð Bjarmason wrote: > > > On Tue, Nov 06 2018, Johannes Schindelin wrote: > > > > > On Mon, 5 Nov 2018, Ævar Arnfjörð Bjarmason wrote: > > > > > >> On Mon, Nov 05 2018, Eric Sunshine wrote: > > >> > > >> > On Mon, Nov 5, 2018 at 3:07 PM Ævar Arnfjörð Bjarmason > > >> > wrote: > > >> >> Add a --no-patch option which shows which changes got removed, added > > >> >> or moved etc., without showing the diff associated with them. > > >> > > > >> > This option existed in the very first version[1] of range-diff (then > > >> > called branch-diff) implemented by Dscho, although it was called > > >> > --no-patches (with an "es"), which it inherited from tbdiff. I think > > >> > someone (possibly me) pointed out that --no-patch (sans "es") would be > > >> > more consistent with existing Git options. I don't recall why Dscho > > >> > removed the option during the re-rolls, but the explanation may be in > > >> > that thread. > > >> > > >> Thanks for digging. Big thread, not going to re-read it now. I'd just > > >> like to have this. > > > > > > In my hands, the well-documented `-s` option works (see e.g. > > > https://git-scm.com/docs/git-diff#git-diff--s), although I have to admit > > > that the `git-range-diff` manual does not talk about the diff-options. > > > > > > And for the record, for me, `git range-diff A...B --no-patch` *already* > > > works. > > > > Neither of those works for me without my patch. E.g. > > > > ./git-range-diff -s 711aaa392f...a5ba8f2101 > > ./git-range-diff --no-patch 711aaa392f...a5ba8f2101 > > > > This is on current next, 2.19.1.1182.g4ecb1133ce. What version are you > > on? > > I tried it with git version 2.19.0.windows.1. > > To verify, I repeated this with `next` (git version > 2.19.1.1215.g8438c0b2453a): > > ./git range-diff -s 711aaa392f...a5ba8f2101 > fatal: unrecognized argument: --output-indicator-new=> > error: could not parse log for 'a5ba8f2101..711aaa392f' > > Which means that something broke rather dramatically between > v2.19.0.windows.1 and 8438c0b2453a. Nevermind, this was solved by passing `--exec-path=$PWD`. And *now* I can reproduce your finding. Ciao, Dscho
Re: [PATCH] range-diff: add a --no-patch option to show a summary
Hi Ævar, On Tue, 6 Nov 2018, Ævar Arnfjörð Bjarmason wrote: > On Tue, Nov 06 2018, Johannes Schindelin wrote: > > > On Mon, 5 Nov 2018, Ævar Arnfjörð Bjarmason wrote: > > > >> On Mon, Nov 05 2018, Eric Sunshine wrote: > >> > >> > On Mon, Nov 5, 2018 at 3:07 PM Ævar Arnfjörð Bjarmason > >> > wrote: > >> >> Add a --no-patch option which shows which changes got removed, added > >> >> or moved etc., without showing the diff associated with them. > >> > > >> > This option existed in the very first version[1] of range-diff (then > >> > called branch-diff) implemented by Dscho, although it was called > >> > --no-patches (with an "es"), which it inherited from tbdiff. I think > >> > someone (possibly me) pointed out that --no-patch (sans "es") would be > >> > more consistent with existing Git options. I don't recall why Dscho > >> > removed the option during the re-rolls, but the explanation may be in > >> > that thread. > >> > >> Thanks for digging. Big thread, not going to re-read it now. I'd just > >> like to have this. > > > > In my hands, the well-documented `-s` option works (see e.g. > > https://git-scm.com/docs/git-diff#git-diff--s), although I have to admit > > that the `git-range-diff` manual does not talk about the diff-options. > > > > And for the record, for me, `git range-diff A...B --no-patch` *already* > > works. > > Neither of those works for me without my patch. E.g. > > ./git-range-diff -s 711aaa392f...a5ba8f2101 > ./git-range-diff --no-patch 711aaa392f...a5ba8f2101 > > This is on current next, 2.19.1.1182.g4ecb1133ce. What version are you > on? I tried it with git version 2.19.0.windows.1. To verify, I repeated this with `next` (git version 2.19.1.1215.g8438c0b2453a): ./git range-diff -s 711aaa392f...a5ba8f2101 fatal: unrecognized argument: --output-indicator-new=> error: could not parse log for 'a5ba8f2101..711aaa392f' Which means that something broke rather dramatically between v2.19.0.windows.1 and 8438c0b2453a. Ciao, Dscho > > >> > >> > I was also wondering if --summarize or --summary-only might be a > >> > better name, describing the behavior at a higher level, but since > >> > there is precedent for --no-patch (or --no-patches in tbdiff), perhaps > >> > the name is fine as is. > >> > >> I think we should aim to keep a 1=1 mapping between range-diff and > >> log/show options when possible, even though the output might have a > >> slightly different flavor as my 4th paragraph discussing a potential > >> --stat talks about. > >> > >> E.g. I can imagine that range-diff --no-patch --stat --summary would not > >> show the patch, but a stat as described there, plus e.g. a "create > >> mode..." if applicable. > >> > >> This change implements only a tiny fraction of that, but it would be > >> very neat if we supported more stuff, and showed it in range-diff-y way, > >> e.g. some compact format showing: > >> > >> 1 file changed, 3->2 insertions(+), 10->9 deletions(-) > >> create mode 100(6 -> 7)44 new-executable > >> > >> > The patch itself looks okay. > >> > > >> > [1]: > >> > https://public-inbox.org/git/8bc517e35d4842f8d9d98f3b99adb9475d6db2d2.1525361419.git.johannes.schinde...@gmx.de/ > >> >
What's cooking in git.git (Nov 2018, #03; Wed, 7)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. The ones marked with '.' do not appear in any of the integration branches, but I am still holding onto them. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [Graduated to "master"] * en/merge-cleanup-more (2018-10-18) 2 commits (merged to 'next' on 2018-10-26 at c706319c26) + merge-recursive: avoid showing conflicts with merge branch before HEAD + merge-recursive: improve auto-merging messages with path collisions (this branch is used by en/merge-path-collision.) Further clean-up of merge-recursive machinery. * jc/http-curlver-warnings (2018-10-26) 1 commit (merged to 'next' on 2018-10-26 at 870e125cec) + http: give curl version warnings consistently (this branch uses js/mingw-http-ssl; is tangled with nd/config-split.) Warning message fix. * js/mingw-http-ssl (2018-10-26) 3 commits (merged to 'next' on 2018-10-26 at 318e82e101) + http: when using Secure Channel, ignore sslCAInfo by default + http: add support for disabling SSL revocation checks in cURL + http: add support for selecting SSL backends at runtime (this branch is used by jc/http-curlver-warnings and nd/config-split.) On platforms with recent cURL library, http.sslBackend configuration variable can be used to choose a different SSL backend at runtime. The Windows port uses this mechanism to switch between OpenSSL and Secure Channel while talking over the HTTPS protocol. * js/mingw-ns-filetime (2018-10-24) 3 commits (merged to 'next' on 2018-10-29 at 4563a0d9d0) + mingw: implement nanosecond-precision file times + mingw: replace MSVCRT's fstat() with a Win32-based implementation + mingw: factor out code to set stat() data Windows port learned to use nano-second resolution file timestamps. * js/remote-archive-dwimfix (2018-10-26) 1 commit (merged to 'next' on 2018-10-26 at f5bf6946bd) + archive: initialize archivers earlier The logic to determine the archive type "git archive" uses did not correctly kick in for "git archive --remote", which has been corrected. * js/shallow-and-fetch-prune (2018-10-25) 3 commits (merged to 'next' on 2018-10-26 at 93b7196560) + repack -ad: prune the list of shallow commits + shallow: offer to prune only non-existing entries + repack: point out a bug handling stale shallow info "git repack" in a shallow clone did not correctly update the shallow points in the repository, leading to a repository that does not pass fsck. * jt/upload-pack-v2-fix-shallow (2018-10-19) 3 commits (merged to 'next' on 2018-10-29 at d9010b3c7b) + upload-pack: clear flags before each v2 request + upload-pack: make want_obj not global + upload-pack: make have_obj not global "git fetch" over protocol v2 into a shallow repository failed to fetch full history behind a new tip of history that was diverged before the cut-off point of the history that was previously fetched shallowly. * jw/send-email-no-auth (2018-10-23) 1 commit (merged to 'next' on 2018-10-29 at a3fbbdb889) + send-email: explicitly disable authentication "git send-email" learned to disable SMTP authentication via the "--smtp-auth=none" option, even when the smtp username is given (which turns the authentication on by default). * md/exclude-promisor-objects-fix (2018-10-23) 2 commits (merged to 'next' on 2018-10-29 at fb36a5dcbe) + exclude-promisor-objects: declare when option is allowed + Documentation/git-log.txt: do not show --exclude-promisor-objects Operations on promisor objects make sense in the context of only a small subset of the commands that internally use the revisions machinery, but the "--exclude-promisor-objects" option were taken and led to nonsense results by commands like "log", to which it didn't make much sense. This has been corrected. * mg/gpg-fingerprint (2018-10-23) 3 commits (merged to 'next' on 2018-10-26 at 1e219cb754) + gpg-interface.c: obtain primary key fingerprint as well + gpg-interface.c: support getting key fingerprint via %GF format + gpg-interface.c: use flags to determine key/signer info presence (this branch uses mg/gpg-parse-tighten; is tangled with jc/gpg-cocci-preincr.) New "--pretty=format:" placeholders %GF and %GP that show the GPG key fingerprints have been invented. * mg/gpg-parse-tighten (2018-10-22) 1 commit (merged to 'next' on 2018-10-26 at efdec77193) + gpg-interface.c: detect and reject multiple signatures on commits (this branch is used by jc/gpg-cocci-preincr and mg/gpg-fingerprint.) Detect and reject a signature block that has more than one GPG signature. * nd/completion-negation (2018-10-22) 1 commit (merged to 'next' on 2018-10-29 at 87e73b0c72) + completion: fix __gitcomp_builtin no longer
