* [PATCH v2 0/2] Show HTTP headers of failed requests with GIT_CURL_VERBOSE
From: Masaya Suzuki @ 2018-12-29 19:44 UTC (permalink / raw)
To: masayasuzuki; +Cc: git, jrnieder, peff, sunshine
In-Reply-To: <20181228014720.206443-1-masayasuzuki@google.com>
When GIT_CURL_VERBOSE is set, libcurl produces request/response headers
to stderr. However, if the response is an error response and
CURLOPT_FAILONERROR is set, libcurl stops parsing the response, and it
won't dump the headers. Showing HTTP response headers is useful for
debugging, especially for non-OK responses.
To this end, the caller of libcurl needs to handle HTTP request failures by
themselves. The first patch makes git prepared to handle those failures. The
second patch actually unsets CURLOPT_FAILONERROR.
Masaya Suzuki (2):
Change how HTTP response body is returned
Unset CURLOPT_FAILONERROR
http.c | 103 +++++++++++++++++++----------------
http.h | 1 -
remote-curl.c | 30 ++++++++--
t/lib-httpd/apache.conf | 1 +
t/t5581-http-curl-verbose.sh | 28 ++++++++++
5 files changed, 110 insertions(+), 53 deletions(-)
create mode 100755 t/t5581-http-curl-verbose.sh
--
2.20.1.415.g653613c723-goog
^ permalink raw reply
* [PATCH v2 1/2] Change how HTTP response body is returned
From: Masaya Suzuki @ 2018-12-29 19:44 UTC (permalink / raw)
To: masayasuzuki; +Cc: git, jrnieder, peff, sunshine
In-Reply-To: <20181229194447.157763-1-masayasuzuki@google.com>
This changes the way HTTP response body is returned in
http_request_reauth and post_rpc.
1. http_request_reauth makes up to two requests; one without a
credential and one with a credential. The first request can fail if
it needs a credential. When the keep_error option is specified, the
response to the first request can be written to the HTTP response
body destination. If the response body destination is a string
buffer, it erases the buffer before making the second request. By
introducing http_response_dest, it can handle the case that the
destination is a file handle.
2. post_rpc makes an HTTP request and the response body is directly
written to a file descriptor. This makes it check the HTTP status
code before writing it, and do not write the response body if it's an
error response. It's ok without this check now because post_rpc makes
a request with CURLOPT_FAILONERROR, and libcurl won't call the
callback if the response has an error status code.
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
---
http.c | 99 +++++++++++++++++++++++++++++----------------------
remote-curl.c | 29 ++++++++++++---
2 files changed, 81 insertions(+), 47 deletions(-)
diff --git a/http.c b/http.c
index eacc2a75e..d23417670 100644
--- a/http.c
+++ b/http.c
@@ -165,6 +165,19 @@ static int http_schannel_check_revoke = 1;
*/
static int http_schannel_use_ssl_cainfo;
+/*
+ * Where to store the result of http_request.
+ *
+ * At most one of buffer or file can be non-NULL. The buffer and file are not
+ * allocated by http_request, and the caller is responsible for releasing them.
+ */
+struct http_response_dest {
+ struct strbuf *buffer;
+
+ FILE *file;
+ const char *filename;
+};
+
size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
{
size_t size = eltsize * nmemb;
@@ -1794,12 +1807,8 @@ static void http_opt_request_remainder(CURL *curl, off_t pos)
curl_easy_setopt(curl, CURLOPT_RANGE, buf);
}
-/* http_request() targets */
-#define HTTP_REQUEST_STRBUF 0
-#define HTTP_REQUEST_FILE 1
-
static int http_request(const char *url,
- void *result, int target,
+ struct http_response_dest *dest,
const struct http_get_options *options)
{
struct active_request_slot *slot;
@@ -1812,21 +1821,23 @@ static int http_request(const char *url,
slot = get_active_slot();
curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);
- if (result == NULL) {
- curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1);
- } else {
+ if (dest->file) {
+ off_t posn = ftello(dest->file);
curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);
- curl_easy_setopt(slot->curl, CURLOPT_FILE, result);
-
- if (target == HTTP_REQUEST_FILE) {
- off_t posn = ftello(result);
- curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION,
- fwrite);
- if (posn > 0)
- http_opt_request_remainder(slot->curl, posn);
- } else
- curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION,
- fwrite_buffer);
+ curl_easy_setopt(slot->curl, CURLOPT_FILE,
+ dest->file);
+ curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION,
+ fwrite);
+ if (posn > 0)
+ http_opt_request_remainder(slot->curl, posn);
+ } else if (dest->buffer) {
+ curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);
+ curl_easy_setopt(slot->curl, CURLOPT_FILE,
+ dest->buffer);
+ curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION,
+ fwrite_buffer);
+ } else {
+ curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1);
}
accept_language = get_accept_language();
@@ -1930,10 +1941,10 @@ static int update_url_from_redirect(struct strbuf *base,
}
static int http_request_reauth(const char *url,
- void *result, int target,
+ struct http_response_dest *dest,
struct http_get_options *options)
{
- int ret = http_request(url, result, target, options);
+ int ret = http_request(url, dest, options);
if (ret != HTTP_OK && ret != HTTP_REAUTH)
return ret;
@@ -1949,32 +1960,34 @@ static int http_request_reauth(const char *url,
if (ret != HTTP_REAUTH)
return ret;
- /*
- * If we are using KEEP_ERROR, the previous request may have
- * put cruft into our output stream; we should clear it out before
- * making our next request. We only know how to do this for
- * the strbuf case, but that is enough to satisfy current callers.
- */
- if (options && options->keep_error) {
- switch (target) {
- case HTTP_REQUEST_STRBUF:
- strbuf_reset(result);
- break;
- default:
- BUG("HTTP_KEEP_ERROR is only supported with strbufs");
+ if (dest->file) {
+ /*
+ * At this point, the file contains the response body of the
+ * previous request. We need to truncate the file.
+ */
+ FILE *new_file = freopen(dest->filename, "w", dest->file);
+ if (new_file == NULL) {
+ error("Unable to open local file %s", dest->filename);
+ return HTTP_ERROR;
}
+ dest->file = new_file;
+ } else if (dest->buffer) {
+ strbuf_reset(dest->buffer);
}
credential_fill(&http_auth);
- return http_request(url, result, target, options);
+ return http_request(url, dest, options);
}
int http_get_strbuf(const char *url,
- struct strbuf *result,
+ struct strbuf *dest_buffer,
struct http_get_options *options)
{
- return http_request_reauth(url, result, HTTP_REQUEST_STRBUF, options);
+ struct http_response_dest dest;
+ dest.file = NULL;
+ dest.buffer = dest_buffer;
+ return http_request_reauth(url, &dest, options);
}
/*
@@ -1988,18 +2001,20 @@ static int http_get_file(const char *url, const char *filename,
{
int ret;
struct strbuf tmpfile = STRBUF_INIT;
- FILE *result;
+ struct http_response_dest dest;
strbuf_addf(&tmpfile, "%s.temp", filename);
- result = fopen(tmpfile.buf, "a");
- if (!result) {
+ dest.buffer = NULL;
+ dest.file = fopen(tmpfile.buf, "a");
+ if (!dest.file) {
error("Unable to open local file %s", tmpfile.buf);
ret = HTTP_ERROR;
goto cleanup;
}
+ dest.filename = tmpfile.buf;
- ret = http_request_reauth(url, result, HTTP_REQUEST_FILE, options);
- fclose(result);
+ ret = http_request_reauth(url, &dest, options);
+ fclose(dest.file);
if (ret == HTTP_OK && finalize_object_file(tmpfile.buf, filename))
ret = HTTP_ERROR;
diff --git a/remote-curl.c b/remote-curl.c
index 1220dffcd..48656bf18 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -546,14 +546,31 @@ static curlioerr rpc_ioctl(CURL *handle, int cmd, void *clientp)
}
#endif
+struct rpc_in_data {
+ struct rpc_state *rpc;
+ struct active_request_slot *slot;
+};
+
+/*
+ * A callback for CURLOPT_WRITEFUNCTION. The return value is the bytes consumed
+ * from ptr.
+ */
static size_t rpc_in(char *ptr, size_t eltsize,
size_t nmemb, void *buffer_)
{
size_t size = eltsize * nmemb;
- struct rpc_state *rpc = buffer_;
+ struct rpc_in_data *data = buffer_;
+ long response_code;
+
+ if (curl_easy_getinfo(data->slot->curl, CURLINFO_RESPONSE_CODE,
+ &response_code) != CURLE_OK)
+ return size;
+ if (response_code != 200)
+ return size;
+
if (size)
- rpc->any_written = 1;
- write_or_die(rpc->in, ptr, size);
+ data->rpc->any_written = 1;
+ write_or_die(data->rpc->in, ptr, size);
return size;
}
@@ -633,6 +650,7 @@ static int post_rpc(struct rpc_state *rpc)
size_t gzip_size = 0;
int err, large_request = 0;
int needs_100_continue = 0;
+ struct rpc_in_data rpc_in_data;
/* Try to load the entire request, if we can fit it into the
* allocated buffer space we can use HTTP/1.0 and avoid the
@@ -765,8 +783,9 @@ static int post_rpc(struct rpc_state *rpc)
curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, headers);
curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, rpc_in);
- curl_easy_setopt(slot->curl, CURLOPT_FILE, rpc);
-
+ rpc_in_data.rpc = rpc;
+ rpc_in_data.slot = slot;
+ curl_easy_setopt(slot->curl, CURLOPT_FILE, &rpc_in_data);
rpc->any_written = 0;
err = run_slot(slot, NULL);
--
2.20.1.415.g653613c723-goog
^ permalink raw reply related
* [PATCH v2 2/2] Unset CURLOPT_FAILONERROR
From: Masaya Suzuki @ 2018-12-29 19:44 UTC (permalink / raw)
To: masayasuzuki; +Cc: git, jrnieder, peff, sunshine
In-Reply-To: <20181229194447.157763-1-masayasuzuki@google.com>
When GIT_CURL_VERBOSE is set, libcurl produces request/response headers
to stderr. However, if the response is an error response and
CURLOPT_FAILONERROR is set, libcurl stops parsing the response, and it
won't dump the headers. Showing HTTP response headers is useful for
debugging, especially for non-OK responses.
This is substantially same as setting http_options.keep_error to all
requests. Hence, remove this option.
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
---
http.c | 4 ----
http.h | 1 -
remote-curl.c | 1 -
t/lib-httpd/apache.conf | 1 +
t/t5581-http-curl-verbose.sh | 28 ++++++++++++++++++++++++++++
5 files changed, 29 insertions(+), 6 deletions(-)
create mode 100755 t/t5581-http-curl-verbose.sh
diff --git a/http.c b/http.c
index d23417670..8f8101da3 100644
--- a/http.c
+++ b/http.c
@@ -1269,7 +1269,6 @@ struct active_request_slot *get_active_slot(void)
curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, NULL);
curl_easy_setopt(slot->curl, CURLOPT_UPLOAD, 0);
curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);
- curl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 1);
curl_easy_setopt(slot->curl, CURLOPT_RANGE, NULL);
/*
@@ -1848,8 +1847,6 @@ static int http_request(const char *url,
strbuf_addstr(&buf, "Pragma:");
if (options && options->no_cache)
strbuf_addstr(&buf, " no-cache");
- if (options && options->keep_error)
- curl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 0);
if (options && options->initial_request &&
http_follow_config == HTTP_FOLLOW_INITIAL)
curl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 1);
@@ -2415,7 +2412,6 @@ struct http_object_request *new_http_object_request(const char *base_url,
freq->slot = get_active_slot();
curl_easy_setopt(freq->slot->curl, CURLOPT_FILE, freq);
- curl_easy_setopt(freq->slot->curl, CURLOPT_FAILONERROR, 0);
curl_easy_setopt(freq->slot->curl, CURLOPT_WRITEFUNCTION, fwrite_sha1_file);
curl_easy_setopt(freq->slot->curl, CURLOPT_ERRORBUFFER, freq->errorstr);
curl_easy_setopt(freq->slot->curl, CURLOPT_URL, freq->url);
diff --git a/http.h b/http.h
index d305ca1dc..eebf40688 100644
--- a/http.h
+++ b/http.h
@@ -146,7 +146,6 @@ extern char *get_remote_object_url(const char *url, const char *hex,
/* Options for http_get_*() */
struct http_get_options {
unsigned no_cache:1,
- keep_error:1,
initial_request:1;
/* If non-NULL, returns the content-type of the response. */
diff --git a/remote-curl.c b/remote-curl.c
index 48656bf18..43e7a1d80 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -380,7 +380,6 @@ static struct discovery *discover_refs(const char *service, int for_push)
http_options.extra_headers = &extra_headers;
http_options.initial_request = 1;
http_options.no_cache = 1;
- http_options.keep_error = 1;
http_ret = http_get_strbuf(refs_url.buf, &buffer, &http_options);
switch (http_ret) {
diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index 581c010d8..cc4b87507 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -115,6 +115,7 @@ Alias /auth/dumb/ www/auth/dumb/
SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}
SetEnv GIT_HTTP_EXPORT_ALL
</LocationMatch>
+ScriptAliasMatch /error_git_upload_pack/(.*)/git-upload-pack error.sh/
ScriptAliasMatch /smart_*[^/]*/(.*) ${GIT_EXEC_PATH}/git-http-backend/$1
ScriptAlias /broken_smart/ broken-smart-http.sh/
ScriptAlias /error/ error.sh/
diff --git a/t/t5581-http-curl-verbose.sh b/t/t5581-http-curl-verbose.sh
new file mode 100755
index 000000000..73148eeba
--- /dev/null
+++ b/t/t5581-http-curl-verbose.sh
@@ -0,0 +1,28 @@
+#!/bin/sh
+
+test_description='test GIT_CURL_VERBOSE'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-httpd.sh
+start_httpd
+
+test_expect_success 'setup repository' '
+ mkdir "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+ git -C "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" --bare init &&
+ git config push.default matching &&
+ echo content >file &&
+ git add file &&
+ git commit -m one &&
+ git remote add public "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+ git push public master:master
+'
+
+test_expect_success 'failure in git-upload-pack is shown' '
+ test_might_fail env GIT_CURL_VERBOSE=1 \
+ git clone "$HTTPD_URL/error_git_upload_pack/smart/repo.git" \
+ 2>curl_log &&
+ cat curl_log | grep "< HTTP/1.1 500 Intentional Breakage"
+'
+
+stop_httpd
+
+test_done
--
2.20.1.415.g653613c723-goog
^ permalink raw reply related
* Re: [PATCH] Use packet_reader instead of packet_read_line
From: Masaya Suzuki @ 2018-12-29 20:05 UTC (permalink / raw)
To: Git Mailing List
In-Reply-To: <20181227065210.60817-1-masayasuzuki@google.com>
Sorry. This is really broken. I'll fix the tests and send another version.
^ permalink raw reply
* [PATCH v2 0/2] Accept error packets in any context
From: Masaya Suzuki @ 2018-12-29 21:19 UTC (permalink / raw)
To: masayasuzuki; +Cc: git, peff, steadmon
In-Reply-To: <reply-to=20181127045301.103807-1-masayasuzuki@google.com>
This makes it possible for servers to send an error message back to clients in
an arbitrary situation.
The first patch was originally sent in [1]. This version includes some fix.
The second patch was originally sent in [2]. Later, this was cherry-picked in
[3]. In the discussion in [3], we agreed that this error packet handling should
be done only against the Git pack protocol handling code. With this agreement,
the patch series sent in [3] is abandoned (according to [4]). This is a patch
series based on that agreement on limiting the error packet handling.
[1]: https://public-inbox.org/git/20181227065210.60817-1-masayasuzuki@google.com/
[2]: https://public-inbox.org/git/20181127045301.103807-1-masayasuzuki@google.com/
[3]: https://public-inbox.org/git/df7d3659ae5f11d163f1e992f3b9403be709ddb7.1544572142.git.steadmon@google.com/
[4]: https://public-inbox.org/git/20181213221826.GE37614@google.com/
Masaya Suzuki (2):
Use packet_reader instead of packet_read_line
pack-protocol.txt: accept error packets in any context
Documentation/technical/pack-protocol.txt | 20 +++----
builtin/archive.c | 19 +++----
builtin/fetch-pack.c | 3 +-
builtin/receive-pack.c | 62 +++++++++++----------
builtin/send-pack.c | 3 +-
connect.c | 3 --
fetch-pack.c | 65 +++++++++++++----------
pkt-line.c | 4 ++
pkt-line.h | 8 ++-
remote-curl.c | 29 ++++++----
send-pack.c | 39 +++++++-------
serve.c | 5 +-
t/t5703-upload-pack-ref-in-want.sh | 4 +-
transport.c | 3 +-
upload-pack.c | 40 +++++++-------
15 files changed, 174 insertions(+), 133 deletions(-)
--
2.20.1.415.g653613c723-goog
^ permalink raw reply
* [PATCH v2 1/2] Use packet_reader instead of packet_read_line
From: Masaya Suzuki @ 2018-12-29 21:19 UTC (permalink / raw)
To: masayasuzuki; +Cc: git, peff, steadmon
In-Reply-To: <20181229211915.161686-1-masayasuzuki@google.com>
By using and sharing a packet_reader while handling a Git pack protocol
request, the same reader option is used throughout the code. This makes
it easy to set a reader option to the request parsing code.
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
---
builtin/archive.c | 19 ++++++-------
builtin/receive-pack.c | 60 +++++++++++++++++++++--------------------
fetch-pack.c | 61 +++++++++++++++++++++++-------------------
remote-curl.c | 22 ++++++++++-----
send-pack.c | 37 ++++++++++++-------------
upload-pack.c | 38 +++++++++++++-------------
6 files changed, 129 insertions(+), 108 deletions(-)
diff --git a/builtin/archive.c b/builtin/archive.c
index d2455237c..2fe1f05ca 100644
--- a/builtin/archive.c
+++ b/builtin/archive.c
@@ -27,10 +27,10 @@ static int run_remote_archiver(int argc, const char **argv,
const char *remote, const char *exec,
const char *name_hint)
{
- char *buf;
int fd[2], i, rv;
struct transport *transport;
struct remote *_remote;
+ struct packet_reader reader;
_remote = remote_get(remote);
if (!_remote->url[0])
@@ -53,18 +53,19 @@ static int run_remote_archiver(int argc, const char **argv,
packet_write_fmt(fd[1], "argument %s\n", argv[i]);
packet_flush(fd[1]);
- buf = packet_read_line(fd[0], NULL);
- if (!buf)
+ packet_reader_init(&reader, fd[0], NULL, 0, PACKET_READ_CHOMP_NEWLINE);
+
+ if (packet_reader_read(&reader) != PACKET_READ_NORMAL)
die(_("git archive: expected ACK/NAK, got a flush packet"));
- if (strcmp(buf, "ACK")) {
- if (starts_with(buf, "NACK "))
- die(_("git archive: NACK %s"), buf + 5);
- if (starts_with(buf, "ERR "))
- die(_("remote error: %s"), buf + 4);
+ if (strcmp(reader.line, "ACK")) {
+ if (starts_with(reader.line, "NACK "))
+ die(_("git archive: NACK %s"), reader.line + 5);
+ if (starts_with(reader.line, "ERR "))
+ die(_("remote error: %s"), reader.line + 4);
die(_("git archive: protocol error"));
}
- if (packet_read_line(fd[0], NULL))
+ if (packet_reader_read(&reader) != PACKET_READ_FLUSH)
die(_("git archive: expected a flush"));
/* Now, start reading from fd[0] and spit it out to stdout */
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 33187bd8e..81cc07370 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1569,30 +1569,29 @@ static void queue_commands_from_cert(struct command **tail,
}
}
-static struct command *read_head_info(struct oid_array *shallow)
+static struct command *read_head_info(struct packet_reader *reader,
+ struct oid_array *shallow)
{
struct command *commands = NULL;
struct command **p = &commands;
for (;;) {
- char *line;
- int len, linelen;
+ int linelen;
- line = packet_read_line(0, &len);
- if (!line)
+ if (packet_reader_read(reader) != PACKET_READ_NORMAL)
break;
- if (len > 8 && starts_with(line, "shallow ")) {
+ if (reader->pktlen > 8 && starts_with(reader->line, "shallow ")) {
struct object_id oid;
- if (get_oid_hex(line + 8, &oid))
+ if (get_oid_hex(reader->line + 8, &oid))
die("protocol error: expected shallow sha, got '%s'",
- line + 8);
+ reader->line + 8);
oid_array_append(shallow, &oid);
continue;
}
- linelen = strlen(line);
- if (linelen < len) {
- const char *feature_list = line + linelen + 1;
+ linelen = strlen(reader->line);
+ if (linelen < reader->pktlen) {
+ const char *feature_list = reader->line + linelen + 1;
if (parse_feature_request(feature_list, "report-status"))
report_status = 1;
if (parse_feature_request(feature_list, "side-band-64k"))
@@ -1607,28 +1606,32 @@ static struct command *read_head_info(struct oid_array *shallow)
use_push_options = 1;
}
- if (!strcmp(line, "push-cert")) {
+ if (!strcmp(reader->line, "push-cert")) {
int true_flush = 0;
- char certbuf[1024];
+ int saved_options = reader->options;
+ reader->options &= ~PACKET_READ_CHOMP_NEWLINE;
for (;;) {
- len = packet_read(0, NULL, NULL,
- certbuf, sizeof(certbuf), 0);
- if (!len) {
+ packet_reader_read(reader);
+ if (reader->status == PACKET_READ_FLUSH) {
true_flush = 1;
break;
}
- if (!strcmp(certbuf, "push-cert-end\n"))
+ if (reader->status != PACKET_READ_NORMAL) {
+ die("protocol error: got an unexpected packet");
+ }
+ if (!strcmp(reader->line, "push-cert-end\n"))
break; /* end of cert */
- strbuf_addstr(&push_cert, certbuf);
+ strbuf_addstr(&push_cert, reader->line);
}
+ reader->options = saved_options;
if (true_flush)
break;
continue;
}
- p = queue_command(p, line, linelen);
+ p = queue_command(p, reader->line, linelen);
}
if (push_cert.len)
@@ -1637,18 +1640,14 @@ static struct command *read_head_info(struct oid_array *shallow)
return commands;
}
-static void read_push_options(struct string_list *options)
+static void read_push_options(struct packet_reader *reader,
+ struct string_list *options)
{
while (1) {
- char *line;
- int len;
-
- line = packet_read_line(0, &len);
-
- if (!line)
+ if (packet_reader_read(reader) != PACKET_READ_NORMAL)
break;
- string_list_append(options, line);
+ string_list_append(options, reader->line);
}
}
@@ -1924,6 +1923,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
struct oid_array shallow = OID_ARRAY_INIT;
struct oid_array ref = OID_ARRAY_INIT;
struct shallow_info si;
+ struct packet_reader reader;
struct option options[] = {
OPT__QUIET(&quiet, N_("quiet")),
@@ -1986,12 +1986,14 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
if (advertise_refs)
return 0;
- if ((commands = read_head_info(&shallow)) != NULL) {
+ packet_reader_init(&reader, 0, NULL, 0, PACKET_READ_CHOMP_NEWLINE);
+
+ if ((commands = read_head_info(&reader, &shallow)) != NULL) {
const char *unpack_status = NULL;
struct string_list push_options = STRING_LIST_INIT_DUP;
if (use_push_options)
- read_push_options(&push_options);
+ read_push_options(&reader, &push_options);
if (!check_cert_push_options(&push_options)) {
struct command *cmd;
for (cmd = commands; cmd; cmd = cmd->next)
diff --git a/fetch-pack.c b/fetch-pack.c
index 9691046e6..86790b9bb 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -135,38 +135,42 @@ enum ack_type {
ACK_ready
};
-static void consume_shallow_list(struct fetch_pack_args *args, int fd)
+static void consume_shallow_list(struct fetch_pack_args *args,
+ struct packet_reader *reader)
{
if (args->stateless_rpc && args->deepen) {
/* If we sent a depth we will get back "duplicate"
* shallow and unshallow commands every time there
* is a block of have lines exchanged.
*/
- char *line;
- while ((line = packet_read_line(fd, NULL))) {
- if (starts_with(line, "shallow "))
+ while (packet_reader_read(reader) == PACKET_READ_NORMAL) {
+ if (starts_with(reader->line, "shallow "))
continue;
- if (starts_with(line, "unshallow "))
+ if (starts_with(reader->line, "unshallow "))
continue;
die(_("git fetch-pack: expected shallow list"));
}
+ if (reader->status != PACKET_READ_FLUSH)
+ die(_("git fetch-pack: expected a flush packet after shallow list"));
}
}
-static enum ack_type get_ack(int fd, struct object_id *result_oid)
+static enum ack_type get_ack(struct packet_reader *reader,
+ struct object_id *result_oid)
{
int len;
- char *line = packet_read_line(fd, &len);
const char *arg;
- if (!line)
+ if (packet_reader_read(reader) != PACKET_READ_NORMAL)
die(_("git fetch-pack: expected ACK/NAK, got a flush packet"));
- if (!strcmp(line, "NAK"))
+ len = reader->pktlen;
+
+ if (!strcmp(reader->line, "NAK"))
return NAK;
- if (skip_prefix(line, "ACK ", &arg)) {
+ if (skip_prefix(reader->line, "ACK ", &arg)) {
if (!get_oid_hex(arg, result_oid)) {
arg += 40;
- len -= arg - line;
+ len -= arg - reader->line;
if (len < 1)
return ACK;
if (strstr(arg, "continue"))
@@ -178,9 +182,9 @@ static enum ack_type get_ack(int fd, struct object_id *result_oid)
return ACK;
}
}
- if (skip_prefix(line, "ERR ", &arg))
+ if (skip_prefix(reader->line, "ERR ", &arg))
die(_("remote error: %s"), arg);
- die(_("git fetch-pack: expected ACK/NAK, got '%s'"), line);
+ die(_("git fetch-pack: expected ACK/NAK, got '%s'"), reader->line);
}
static void send_request(struct fetch_pack_args *args,
@@ -248,10 +252,14 @@ static int find_common(struct fetch_negotiator *negotiator,
int got_ready = 0;
struct strbuf req_buf = STRBUF_INIT;
size_t state_len = 0;
+ struct packet_reader reader;
if (args->stateless_rpc && multi_ack == 1)
die(_("--stateless-rpc requires multi_ack_detailed"));
+ packet_reader_init(&reader, fd[0], NULL, 0,
+ PACKET_READ_CHOMP_NEWLINE);
+
if (!args->no_dependents) {
mark_tips(negotiator, args->negotiation_tips);
for_each_cached_alternate(negotiator, insert_one_alternate_object);
@@ -336,31 +344,30 @@ static int find_common(struct fetch_negotiator *negotiator,
state_len = req_buf.len;
if (args->deepen) {
- char *line;
const char *arg;
struct object_id oid;
send_request(args, fd[1], &req_buf);
- while ((line = packet_read_line(fd[0], NULL))) {
- if (skip_prefix(line, "shallow ", &arg)) {
+ while (packet_reader_read(&reader) == PACKET_READ_NORMAL) {
+ if (skip_prefix(reader.line, "shallow ", &arg)) {
if (get_oid_hex(arg, &oid))
- die(_("invalid shallow line: %s"), line);
+ die(_("invalid shallow line: %s"), reader.line);
register_shallow(the_repository, &oid);
continue;
}
- if (skip_prefix(line, "unshallow ", &arg)) {
+ if (skip_prefix(reader.line, "unshallow ", &arg)) {
if (get_oid_hex(arg, &oid))
- die(_("invalid unshallow line: %s"), line);
+ die(_("invalid unshallow line: %s"), reader.line);
if (!lookup_object(the_repository, oid.hash))
- die(_("object not found: %s"), line);
+ die(_("object not found: %s"), reader.line);
/* make sure that it is parsed as shallow */
if (!parse_object(the_repository, &oid))
- die(_("error in object: %s"), line);
+ die(_("error in object: %s"), reader.line);
if (unregister_shallow(&oid))
- die(_("no shallow found: %s"), line);
+ die(_("no shallow found: %s"), reader.line);
continue;
}
- die(_("expected shallow/unshallow, got %s"), line);
+ die(_("expected shallow/unshallow, got %s"), reader.line);
}
} else if (!args->stateless_rpc)
send_request(args, fd[1], &req_buf);
@@ -397,9 +404,9 @@ static int find_common(struct fetch_negotiator *negotiator,
if (!args->stateless_rpc && count == INITIAL_FLUSH)
continue;
- consume_shallow_list(args, fd[0]);
+ consume_shallow_list(args, &reader);
do {
- ack = get_ack(fd[0], result_oid);
+ ack = get_ack(&reader, result_oid);
if (ack)
print_verbose(args, _("got %s %d %s"), "ack",
ack, oid_to_hex(result_oid));
@@ -469,9 +476,9 @@ static int find_common(struct fetch_negotiator *negotiator,
strbuf_release(&req_buf);
if (!got_ready || !no_done)
- consume_shallow_list(args, fd[0]);
+ consume_shallow_list(args, &reader);
while (flushes || multi_ack) {
- int ack = get_ack(fd[0], result_oid);
+ int ack = get_ack(&reader, result_oid);
if (ack) {
print_verbose(args, _("got %s (%d) %s"), "ack",
ack, oid_to_hex(result_oid));
diff --git a/remote-curl.c b/remote-curl.c
index 1220dffcd..bbd9ba0f3 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -409,28 +409,36 @@ static struct discovery *discover_refs(const char *service, int for_push)
if (maybe_smart &&
(5 <= last->len && last->buf[4] == '#') &&
!strbuf_cmp(&exp, &type)) {
- char *line;
+ struct packet_reader reader;
+ packet_reader_init(&reader, -1, last->buf, last->len,
+ PACKET_READ_CHOMP_NEWLINE);
/*
* smart HTTP response; validate that the service
* pkt-line matches our request.
*/
- line = packet_read_line_buf(&last->buf, &last->len, NULL);
- if (!line)
+ if (packet_reader_read(&reader) != PACKET_READ_NORMAL)
die("invalid server response; expected service, got flush packet");
strbuf_reset(&exp);
strbuf_addf(&exp, "# service=%s", service);
- if (strcmp(line, exp.buf))
- die("invalid server response; got '%s'", line);
+ if (strcmp(reader.line, exp.buf))
+ die("invalid server response; got '%s'", reader.line);
strbuf_release(&exp);
/* The header can include additional metadata lines, up
* until a packet flush marker. Ignore these now, but
* in the future we might start to scan them.
*/
- while (packet_read_line_buf(&last->buf, &last->len, NULL))
- ;
+ for (;;) {
+ packet_reader_read(&reader);
+ if (reader.pktlen <= 0) {
+ break;
+ }
+ }
+
+ last->buf = reader.src_buffer;
+ last->len = reader.src_len;
last->proto_git = 1;
} else if (maybe_smart &&
diff --git a/send-pack.c b/send-pack.c
index f69268677..913645046 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -135,38 +135,36 @@ static int pack_objects(int fd, struct ref *refs, struct oid_array *extra, struc
return 0;
}
-static int receive_unpack_status(int in)
+static int receive_unpack_status(struct packet_reader *reader)
{
- const char *line = packet_read_line(in, NULL);
- if (!line)
+ if (packet_reader_read(reader) != PACKET_READ_NORMAL)
return error(_("unexpected flush packet while reading remote unpack status"));
- if (!skip_prefix(line, "unpack ", &line))
- return error(_("unable to parse remote unpack status: %s"), line);
- if (strcmp(line, "ok"))
- return error(_("remote unpack failed: %s"), line);
+ if (!skip_prefix(reader->line, "unpack ", &reader->line))
+ return error(_("unable to parse remote unpack status: %s"), reader->line);
+ if (strcmp(reader->line, "ok"))
+ return error(_("remote unpack failed: %s"), reader->line);
return 0;
}
-static int receive_status(int in, struct ref *refs)
+static int receive_status(struct packet_reader *reader, struct ref *refs)
{
struct ref *hint;
int ret;
hint = NULL;
- ret = receive_unpack_status(in);
+ ret = receive_unpack_status(reader);
while (1) {
- char *refname;
+ const char *refname;
char *msg;
- char *line = packet_read_line(in, NULL);
- if (!line)
+ if (packet_reader_read(reader) != PACKET_READ_NORMAL)
break;
- if (!starts_with(line, "ok ") && !starts_with(line, "ng ")) {
- error("invalid ref status from remote: %s", line);
+ if (!starts_with(reader->line, "ok ") && !starts_with(reader->line, "ng ")) {
+ error("invalid ref status from remote: %s", reader->line);
ret = -1;
break;
}
- refname = line + 3;
+ refname = reader->line + 3;
msg = strchr(refname, ' ');
if (msg)
*msg++ = '\0';
@@ -187,7 +185,7 @@ static int receive_status(int in, struct ref *refs)
continue;
}
- if (line[0] == 'o' && line[1] == 'k')
+ if (reader->line[0] == 'o' && reader->line[1] == 'k')
hint->status = REF_STATUS_OK;
else {
hint->status = REF_STATUS_REMOTE_REJECT;
@@ -390,6 +388,7 @@ int send_pack(struct send_pack_args *args,
int ret;
struct async demux;
const char *push_cert_nonce = NULL;
+ struct packet_reader reader;
/* Does the other end support the reporting? */
if (server_supports("report-status"))
@@ -559,6 +558,8 @@ int send_pack(struct send_pack_args *args,
in = demux.out;
}
+ packet_reader_init(&reader, in, NULL, 0, PACKET_READ_CHOMP_NEWLINE);
+
if (need_pack_data && cmds_sent) {
if (pack_objects(out, remote_refs, extra_have, args) < 0) {
for (ref = remote_refs; ref; ref = ref->next)
@@ -573,7 +574,7 @@ int send_pack(struct send_pack_args *args,
* are failing, and just want the error() side effects.
*/
if (status_report)
- receive_unpack_status(in);
+ receive_unpack_status(&reader);
if (use_sideband) {
close(demux.out);
@@ -590,7 +591,7 @@ int send_pack(struct send_pack_args *args,
packet_flush(out);
if (status_report && cmds_sent)
- ret = receive_status(in, remote_refs);
+ ret = receive_status(&reader, remote_refs);
else
ret = 0;
if (args->stateless_rpc)
diff --git a/upload-pack.c b/upload-pack.c
index 5e81f1ff2..1638825ee 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -354,7 +354,8 @@ static int ok_to_give_up(const struct object_array *have_obj,
min_generation);
}
-static int get_common_commits(struct object_array *have_obj,
+static int get_common_commits(struct packet_reader *reader,
+ struct object_array *have_obj,
struct object_array *want_obj)
{
struct object_id oid;
@@ -366,12 +367,11 @@ static int get_common_commits(struct object_array *have_obj,
save_commit_buffer = 0;
for (;;) {
- char *line = packet_read_line(0, NULL);
const char *arg;
reset_timeout();
- if (!line) {
+ if (packet_reader_read(reader) != PACKET_READ_NORMAL) {
if (multi_ack == 2 && got_common
&& !got_other && ok_to_give_up(have_obj, want_obj)) {
sent_ready = 1;
@@ -390,7 +390,7 @@ static int get_common_commits(struct object_array *have_obj,
got_other = 0;
continue;
}
- if (skip_prefix(line, "have ", &arg)) {
+ if (skip_prefix(reader->line, "have ", &arg)) {
switch (got_oid(arg, &oid, have_obj)) {
case -1: /* they have what we do not */
got_other = 1;
@@ -416,7 +416,7 @@ static int get_common_commits(struct object_array *have_obj,
}
continue;
}
- if (!strcmp(line, "done")) {
+ if (!strcmp(reader->line, "done")) {
if (have_obj->nr > 0) {
if (multi_ack)
packet_write_fmt(1, "ACK %s\n", last_hex);
@@ -425,7 +425,7 @@ static int get_common_commits(struct object_array *have_obj,
packet_write_fmt(1, "NAK\n");
return -1;
}
- die("git upload-pack: expected SHA1 list, got '%s'", line);
+ die("git upload-pack: expected SHA1 list, got '%s'", reader->line);
}
}
@@ -826,7 +826,7 @@ static int process_deepen_not(const char *line, struct string_list *deepen_not,
return 0;
}
-static void receive_needs(struct object_array *want_obj)
+static void receive_needs(struct packet_reader *reader, struct object_array *want_obj)
{
struct object_array shallows = OBJECT_ARRAY_INIT;
struct string_list deepen_not = STRING_LIST_INIT_DUP;
@@ -840,33 +840,32 @@ static void receive_needs(struct object_array *want_obj)
struct object *o;
const char *features;
struct object_id oid_buf;
- char *line = packet_read_line(0, NULL);
const char *arg;
reset_timeout();
- if (!line)
+ if (packet_reader_read(reader) != PACKET_READ_NORMAL)
break;
- if (process_shallow(line, &shallows))
+ if (process_shallow(reader->line, &shallows))
continue;
- if (process_deepen(line, &depth))
+ if (process_deepen(reader->line, &depth))
continue;
- if (process_deepen_since(line, &deepen_since, &deepen_rev_list))
+ if (process_deepen_since(reader->line, &deepen_since, &deepen_rev_list))
continue;
- if (process_deepen_not(line, &deepen_not, &deepen_rev_list))
+ if (process_deepen_not(reader->line, &deepen_not, &deepen_rev_list))
continue;
- if (skip_prefix(line, "filter ", &arg)) {
+ if (skip_prefix(reader->line, "filter ", &arg)) {
if (!filter_capability_requested)
die("git upload-pack: filtering capability not negotiated");
parse_list_objects_filter(&filter_options, arg);
continue;
}
- if (!skip_prefix(line, "want ", &arg) ||
+ if (!skip_prefix(reader->line, "want ", &arg) ||
parse_oid_hex(arg, &oid_buf, &features))
die("git upload-pack: protocol error, "
- "expected to get object ID, not '%s'", line);
+ "expected to get object ID, not '%s'", reader->line);
if (parse_feature_request(features, "deepen-relative"))
deepen_relative = 1;
@@ -1055,6 +1054,7 @@ void upload_pack(struct upload_pack_options *options)
{
struct string_list symref = STRING_LIST_INIT_DUP;
struct object_array want_obj = OBJECT_ARRAY_INIT;
+ struct packet_reader reader;
stateless_rpc = options->stateless_rpc;
timeout = options->timeout;
@@ -1078,10 +1078,12 @@ void upload_pack(struct upload_pack_options *options)
if (options->advertise_refs)
return;
- receive_needs(&want_obj);
+ packet_reader_init(&reader, 0, NULL, 0, PACKET_READ_CHOMP_NEWLINE);
+
+ receive_needs(&reader, &want_obj);
if (want_obj.nr) {
struct object_array have_obj = OBJECT_ARRAY_INIT;
- get_common_commits(&have_obj, &want_obj);
+ get_common_commits(&reader, &have_obj, &want_obj);
create_pack_file(&have_obj, &want_obj);
}
}
--
2.20.1.415.g653613c723-goog
^ permalink raw reply related
* [PATCH v2 2/2] pack-protocol.txt: accept error packets in any context
From: Masaya Suzuki @ 2018-12-29 21:19 UTC (permalink / raw)
To: masayasuzuki; +Cc: git, peff, steadmon
In-Reply-To: <20181229211915.161686-1-masayasuzuki@google.com>
In the Git pack protocol definition, an error packet may appear only in
a certain context. However, servers can face a runtime error (e.g. I/O
error) at an arbitrary timing. This patch changes the protocol to allow
an error packet to be sent instead of any packet.
Without this protocol spec change, when a server cannot process a
request, there's no way to tell that to a client. Since the server
cannot produce a valid response, it would be forced to cut a connection
without telling why. With this protocol spec change, the server can be
more gentle in this situation. An old client may see these error packets
as an unexpected packet, but this is not worse than having an unexpected
EOF.
Following this protocol spec change, the error packet handling code is
moved to pkt-line.c. Implementation wise, this implementation uses
pkt-line to communicate with a subprocess. Since this is not a part of
Git protocol, it's possible that a packet that is not supposed to be an
error packet is mistakenly parsed as an error packet. This error packet
handling is enabled only for the Git pack protocol parsing code
considering this.
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
---
Documentation/technical/pack-protocol.txt | 20 +++++++++++---------
builtin/archive.c | 6 +++---
builtin/fetch-pack.c | 3 ++-
builtin/receive-pack.c | 4 +++-
builtin/send-pack.c | 3 ++-
connect.c | 3 ---
fetch-pack.c | 8 ++++----
pkt-line.c | 4 ++++
pkt-line.h | 8 ++++++--
remote-curl.c | 9 ++++++---
send-pack.c | 4 +++-
serve.c | 5 +++--
t/t5703-upload-pack-ref-in-want.sh | 4 ++--
transport.c | 3 ++-
upload-pack.c | 4 +++-
15 files changed, 54 insertions(+), 34 deletions(-)
diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt
index 6ac774d5f..7a2375a55 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -22,6 +22,16 @@ protocol-common.txt. When the grammar indicate `PKT-LINE(...)`, unless
otherwise noted the usual pkt-line LF rules apply: the sender SHOULD
include a LF, but the receiver MUST NOT complain if it is not present.
+An error packet is a special pkt-line that contains an error string.
+
+----
+ error-line = PKT-LINE("ERR" SP explanation-text)
+----
+
+Throughout the protocol, where `PKT-LINE(...)` is expected, an error packet MAY
+be sent. Once this packet is sent by a client or a server, the data transfer
+process defined in this protocol is terminated.
+
Transports
----------
There are three transports over which the packfile protocol is
@@ -89,13 +99,6 @@ process on the server side over the Git protocol is this:
"0039git-upload-pack /schacon/gitbook.git\0host=example.com\0" |
nc -v example.com 9418
-If the server refuses the request for some reasons, it could abort
-gracefully with an error message.
-
-----
- error-line = PKT-LINE("ERR" SP explanation-text)
-----
-
SSH Transport
-------------
@@ -398,12 +401,11 @@ from the client).
Then the server will start sending its packfile data.
----
- server-response = *ack_multi ack / nak / error-line
+ server-response = *ack_multi ack / nak
ack_multi = PKT-LINE("ACK" SP obj-id ack_status)
ack_status = "continue" / "common" / "ready"
ack = PKT-LINE("ACK" SP obj-id)
nak = PKT-LINE("NAK")
- error-line = PKT-LINE("ERR" SP explanation-text)
----
A simple clone may look like this (with no 'have' lines):
diff --git a/builtin/archive.c b/builtin/archive.c
index 2fe1f05ca..45d11669a 100644
--- a/builtin/archive.c
+++ b/builtin/archive.c
@@ -53,15 +53,15 @@ static int run_remote_archiver(int argc, const char **argv,
packet_write_fmt(fd[1], "argument %s\n", argv[i]);
packet_flush(fd[1]);
- packet_reader_init(&reader, fd[0], NULL, 0, PACKET_READ_CHOMP_NEWLINE);
+ packet_reader_init(&reader, fd[0], NULL, 0,
+ PACKET_READ_CHOMP_NEWLINE |
+ PACKET_READ_DIE_ON_ERR_PACKET);
if (packet_reader_read(&reader) != PACKET_READ_NORMAL)
die(_("git archive: expected ACK/NAK, got a flush packet"));
if (strcmp(reader.line, "ACK")) {
if (starts_with(reader.line, "NACK "))
die(_("git archive: NACK %s"), reader.line + 5);
- if (starts_with(reader.line, "ERR "))
- die(_("remote error: %s"), reader.line + 4);
die(_("git archive: protocol error"));
}
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 63e69a580..85dbc2af8 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -217,7 +217,8 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
packet_reader_init(&reader, fd[0], NULL, 0,
PACKET_READ_CHOMP_NEWLINE |
- PACKET_READ_GENTLE_ON_EOF);
+ PACKET_READ_GENTLE_ON_EOF |
+ PACKET_READ_DIE_ON_ERR_PACKET);
switch (discover_version(&reader)) {
case protocol_v2:
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 81cc07370..d58b7750b 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1986,7 +1986,9 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
if (advertise_refs)
return 0;
- packet_reader_init(&reader, 0, NULL, 0, PACKET_READ_CHOMP_NEWLINE);
+ packet_reader_init(&reader, 0, NULL, 0,
+ PACKET_READ_CHOMP_NEWLINE |
+ PACKET_READ_DIE_ON_ERR_PACKET);
if ((commands = read_head_info(&reader, &shallow)) != NULL) {
const char *unpack_status = NULL;
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 8e3c7490f..098ebf22d 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -250,7 +250,8 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
packet_reader_init(&reader, fd[0], NULL, 0,
PACKET_READ_CHOMP_NEWLINE |
- PACKET_READ_GENTLE_ON_EOF);
+ PACKET_READ_GENTLE_ON_EOF |
+ PACKET_READ_DIE_ON_ERR_PACKET);
switch (discover_version(&reader)) {
case protocol_v2:
diff --git a/connect.c b/connect.c
index 24281b608..4813f005a 100644
--- a/connect.c
+++ b/connect.c
@@ -296,7 +296,6 @@ struct ref **get_remote_heads(struct packet_reader *reader,
struct ref **orig_list = list;
int len = 0;
enum get_remote_heads_state state = EXPECTING_FIRST_REF;
- const char *arg;
*list = NULL;
@@ -306,8 +305,6 @@ struct ref **get_remote_heads(struct packet_reader *reader,
die_initial_contact(1);
case PACKET_READ_NORMAL:
len = reader->pktlen;
- if (len > 4 && skip_prefix(reader->line, "ERR ", &arg))
- die(_("remote error: %s"), arg);
break;
case PACKET_READ_FLUSH:
state = EXPECTING_DONE;
diff --git a/fetch-pack.c b/fetch-pack.c
index 86790b9bb..3f9626dbf 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -182,8 +182,6 @@ static enum ack_type get_ack(struct packet_reader *reader,
return ACK;
}
}
- if (skip_prefix(reader->line, "ERR ", &arg))
- die(_("remote error: %s"), arg);
die(_("git fetch-pack: expected ACK/NAK, got '%s'"), reader->line);
}
@@ -258,7 +256,8 @@ static int find_common(struct fetch_negotiator *negotiator,
die(_("--stateless-rpc requires multi_ack_detailed"));
packet_reader_init(&reader, fd[0], NULL, 0,
- PACKET_READ_CHOMP_NEWLINE);
+ PACKET_READ_CHOMP_NEWLINE |
+ PACKET_READ_DIE_ON_ERR_PACKET);
if (!args->no_dependents) {
mark_tips(negotiator, args->negotiation_tips);
@@ -1358,7 +1357,8 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
struct fetch_negotiator negotiator;
fetch_negotiator_init(&negotiator, negotiation_algorithm);
packet_reader_init(&reader, fd[0], NULL, 0,
- PACKET_READ_CHOMP_NEWLINE);
+ PACKET_READ_CHOMP_NEWLINE |
+ PACKET_READ_DIE_ON_ERR_PACKET);
while (state != FETCH_DONE) {
switch (state) {
diff --git a/pkt-line.c b/pkt-line.c
index 04d10bbd0..e70ea6d88 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -346,6 +346,10 @@ enum packet_read_status packet_read_with_status(int fd, char **src_buffer,
return PACKET_READ_EOF;
}
+ if ((options & PACKET_READ_DIE_ON_ERR_PACKET) &&
+ starts_with(buffer, "ERR "))
+ die(_("remote error: %s"), buffer + 4);
+
if ((options & PACKET_READ_CHOMP_NEWLINE) &&
len && buffer[len-1] == '\n')
len--;
diff --git a/pkt-line.h b/pkt-line.h
index 5b28d4347..d7e1dbc04 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -62,9 +62,13 @@ int write_packetized_from_buf(const char *src_in, size_t len, int fd_out);
*
* If options contains PACKET_READ_CHOMP_NEWLINE, a trailing newline (if
* present) is removed from the buffer before returning.
+ *
+ * If options contains PACKET_READ_DIE_ON_ERR_PACKET, it dies when it sees an
+ * ERR packet.
*/
-#define PACKET_READ_GENTLE_ON_EOF (1u<<0)
-#define PACKET_READ_CHOMP_NEWLINE (1u<<1)
+#define PACKET_READ_GENTLE_ON_EOF (1u<<0)
+#define PACKET_READ_CHOMP_NEWLINE (1u<<1)
+#define PACKET_READ_DIE_ON_ERR_PACKET (1u<<2)
int packet_read(int fd, char **src_buffer, size_t *src_len, char
*buffer, unsigned size, int options);
diff --git a/remote-curl.c b/remote-curl.c
index bbd9ba0f3..10b50869c 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -204,7 +204,8 @@ static struct ref *parse_git_refs(struct discovery *heads, int for_push)
packet_reader_init(&reader, -1, heads->buf, heads->len,
PACKET_READ_CHOMP_NEWLINE |
- PACKET_READ_GENTLE_ON_EOF);
+ PACKET_READ_GENTLE_ON_EOF |
+ PACKET_READ_DIE_ON_ERR_PACKET);
heads->version = discover_version(&reader);
switch (heads->version) {
@@ -411,7 +412,8 @@ static struct discovery *discover_refs(const char *service, int for_push)
!strbuf_cmp(&exp, &type)) {
struct packet_reader reader;
packet_reader_init(&reader, -1, last->buf, last->len,
- PACKET_READ_CHOMP_NEWLINE);
+ PACKET_READ_CHOMP_NEWLINE |
+ PACKET_READ_DIE_ON_ERR_PACKET);
/*
* smart HTTP response; validate that the service
@@ -1182,7 +1184,8 @@ static void proxy_state_init(struct proxy_state *p, const char *service_name,
p->headers = curl_slist_append(p->headers, buf.buf);
packet_reader_init(&p->reader, p->in, NULL, 0,
- PACKET_READ_GENTLE_ON_EOF);
+ PACKET_READ_GENTLE_ON_EOF |
+ PACKET_READ_DIE_ON_ERR_PACKET);
strbuf_release(&buf);
}
diff --git a/send-pack.c b/send-pack.c
index 913645046..7b9829f16 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -558,7 +558,9 @@ int send_pack(struct send_pack_args *args,
in = demux.out;
}
- packet_reader_init(&reader, in, NULL, 0, PACKET_READ_CHOMP_NEWLINE);
+ packet_reader_init(&reader, in, NULL, 0,
+ PACKET_READ_CHOMP_NEWLINE |
+ PACKET_READ_DIE_ON_ERR_PACKET);
if (need_pack_data && cmds_sent) {
if (pack_objects(out, remote_refs, extra_have, args) < 0) {
diff --git a/serve.c b/serve.c
index bda085f09..317256c1a 100644
--- a/serve.c
+++ b/serve.c
@@ -167,7 +167,8 @@ static int process_request(void)
packet_reader_init(&reader, 0, NULL, 0,
PACKET_READ_CHOMP_NEWLINE |
- PACKET_READ_GENTLE_ON_EOF);
+ PACKET_READ_GENTLE_ON_EOF |
+ PACKET_READ_DIE_ON_ERR_PACKET);
/*
* Check to see if the client closed their end before sending another
@@ -175,7 +176,7 @@ static int process_request(void)
*/
if (packet_reader_peek(&reader) == PACKET_READ_EOF)
return 1;
- reader.options = PACKET_READ_CHOMP_NEWLINE;
+ reader.options &= ~PACKET_READ_GENTLE_ON_EOF;
while (state != PROCESS_REQUEST_DONE) {
switch (packet_reader_peek(&reader)) {
diff --git a/t/t5703-upload-pack-ref-in-want.sh b/t/t5703-upload-pack-ref-in-want.sh
index 3f58f05cb..d2a9d0c12 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
+ grep "fatal: remote error: 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 refs/heads/raster" err
+ grep "fatal: remote error: unknown ref refs/heads/raster" err
'
stop_httpd
diff --git a/transport.c b/transport.c
index 5a74b609f..12db4251c 100644
--- a/transport.c
+++ b/transport.c
@@ -273,7 +273,8 @@ static struct ref *handshake(struct transport *transport, int for_push,
packet_reader_init(&reader, data->fd[0], NULL, 0,
PACKET_READ_CHOMP_NEWLINE |
- PACKET_READ_GENTLE_ON_EOF);
+ PACKET_READ_GENTLE_ON_EOF |
+ PACKET_READ_DIE_ON_ERR_PACKET);
data->version = discover_version(&reader);
switch (data->version) {
diff --git a/upload-pack.c b/upload-pack.c
index 1638825ee..08b547cf0 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -1078,7 +1078,9 @@ void upload_pack(struct upload_pack_options *options)
if (options->advertise_refs)
return;
- packet_reader_init(&reader, 0, NULL, 0, PACKET_READ_CHOMP_NEWLINE);
+ packet_reader_init(&reader, 0, NULL, 0,
+ PACKET_READ_CHOMP_NEWLINE |
+ PACKET_READ_DIE_ON_ERR_PACKET);
receive_needs(&reader, &want_obj);
if (want_obj.nr) {
--
2.20.1.415.g653613c723-goog
^ permalink raw reply related
* Re: [PATCH 2/2] Rebase: Run post-checkout hook on checkout
From: Orgad Shaneh @ 2018-12-29 21:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqimzdl0v4.fsf@gitster-ct.c.googlers.com>
On Sat, Dec 29, 2018 at 12:53 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> orgads@gmail.com writes:
>
> > From: Orgad Shaneh <orgads@gmail.com>
>
> > Re: [PATCH 2/2] Rebase: Run post-checkout hook on checkout
>
> There is no explanation here?
>
> Is this a regression fix (i.e. scripted version of "rebase" used to
> run the hook)? Or a new feature (i.e. no earlier version of
> "rebase" run the hook but you think it ought to run it)?
Added an explanation.
[snip]
>
> > +#define RESET_HEAD_RUN_HOOK (1<<2)
>
> Would it be plausible that the only possible hook that can be run by
> reset_head() function will always be post-checkout and nothing else,
> I wonder? Shouldn't this bit be called *_RUN_POST_CHECKOUT to make
> sure it is specific enough?
Done
[snip]
>
> > +test_expect_success 'post-checkout is triggered on rebase' '
> > + git checkout -b rebase-test master &&
> > + rm -f .git/post-checkout.args &&
>
> Read the title of this whole test script file; it should verify what
> is in the file before removing it.
Why? Other tests already do it. This test is meant for rebase, not
plain checkout.
>
> > + git rebase rebase-on-me &&
> > + read old new flag < .git/post-checkout.args &&
>
> No SP between "<" and ".git/post-checkout.args".
Done
> > + test $old != $new && test $flag = 1 &&
> > + rm -f .git/post-checkout.args
> > +'
>
> Regarding the clean-up of this file, see my review on the previous
> one.
Done
> > +test_expect_success 'post-checkout is triggered on rebase with fast-forward' '
>
> The same comment as above applies to this.
Done
Thanks,
- Orgad
^ permalink raw reply
* Re: [PATCH 1/2] t5403: Refactor
From: Orgad Shaneh @ 2018-12-29 21:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Ramsay Jones
In-Reply-To: <xmqqmuopl1qz.fsf@gitster-ct.c.googlers.com>
On Sat, Dec 29, 2018 at 12:34 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> orgads@gmail.com writes:
>
> > Subject: Re: [PATCH 1/2] t5403: Refactor
>
> Hmph. "Refactor" alone leaves readers wondering "refactor what?",
> "refactor for what?" and "refactor how?". Given that the overfall
> goal this change seeks seems to be to simplify it by losing about 20
> lines, how about justifying it like so?
>
> Subject: t5403: simplify by using a single repository
>
> There is no strong reason to use separate clones to run
> these tests; just use a single test repository prepared
> with more modern test_commit shell helper function.
>
> While at it, replace three "awk '{print $N}'" on the same
> file with shell built-in "read" into three variables.
Done
[snip]
> > + mv .git/hooks-disabled .git/hooks &&
>
> I am not sure why you want to do this---it sends a wrong signal to
> readers saying that you want to use whatever hook that are in the
> moved-away .git/hooks-disabled/ directory. I am guessing that the
> only reason why you do this is because there must be .git/hooks
> directory in order for write_script below to work, so a more
> readable approach would be to "mkdir .git/hooks" instead, no?
Agreed. Done.
> > + write_script .git/hooks/post-checkout <<-\EOF &&
> > + echo $@ >.git/post-checkout.args
>
> A dollar-at inside a shell script that is not in a pair of dq always
> makes readers wonder if the author forgot dq around it or omitted eq
> around it deliberately; avoid it.
>
> In this case, use "$@" (i.e. within dq) would be more friendly to
> readers.
Done.
[snip]
> > + EOF
> > + test_commit one &&
> > + test_commit two &&
> > + test_commit three three
>
> Makes readers wonder why the last one duplicates. Is this because
> you somehow do not want to use three.t as the pathname in a later
> test? "test_commit X" that creates test file X.t is a quite well
> established convention (see "git grep '\.t\>' t/"), by the way.
Done. I wasn't aware of that.
[snip]
> > + test -e .git/post-checkout.args &&
>
> Use "test -f", as you do know you'd be creating a file ("-e"
> succeeds as long as it _exists_, and does not care if it is a file
> or directory or whatever).
Just removed these `test -e` lines. read fails anyway if the file doesn't exist.
> > + read old new flag <.git/post-checkout.args &&
>
> This indeed is much nicer.
Credit goes to Johannes :)
> > + test $old = $new && test $flag = 1 &&
> > + rm -f .git/post-checkout.args
>
> The last one is not a test but a clean-up. If any of the earlier
> step failed (e.g. $old and $new were different), the output file
> would be left behind, resulting in confusing the next test.
>
> Instead, do it like so:
>
> test_expect_success 'title of the test' '
> test_when_finished "rm -f .git/post-checkout.args" &&
> git checkout master &&
> test -f .git/post-checkout.args &&
> read old new flag <.git/post-checkout.args &&
> test $old = $new &&
> test $flag = 1
> '
>
> That is, use test_when_finished() before the step that creates the
> file that may be left behind to arrange that it will be cleaned at
> the end.
>
> This comment on clean-up applies to _all_ tests in this patch that
> has "rm -f .git/post-checkout.args" at the end.
Done
> > test_expect_success 'post-checkout runs as expected ' '
> > - GIT_DIR=clone1/.git git checkout master &&
> > - test -e clone1/.git/post-checkout.args
> > + git checkout master &&
> > + test -e .git/post-checkout.args &&
> > + rm -f .git/post-checkout.args
> > '
>
> Now that the script got so simplified, this one looks even more
> redundant, given that the previous one already checked the same
> "checkout 'master' when already at the tip of 'master'" situation.
>
> Do we still need this one, in other words?
I agree. Removed.
> > test_expect_success 'post-checkout args are correct with git checkout -b ' '
> > - GIT_DIR=clone1/.git git checkout -b new1 &&
> > - old=$(awk "{print \$1}" clone1/.git/post-checkout.args) &&
> > - new=$(awk "{print \$2}" clone1/.git/post-checkout.args) &&
> > - flag=$(awk "{print \$3}" clone1/.git/post-checkout.args) &&
> > - test $old = $new && test $flag = 1
> > + git checkout -b new1 &&
> > + read old new flag <.git/post-checkout.args &&
> > + test $old = $new && test $flag = 1 &&
> > + rm -f .git/post-checkout.args
> > '
>
> This one forgets "did the hook run and create the file" before
> "read", unlike the previous tests. It is not strictly necessary as
> "read" will fail if the file is not there, but it'd be better to be
> consistent.
Made consistent by removing file existence test, and left only read.
> > if test "$(git config --bool core.filemode)" = true; then
>
> This is a tangent but this conditional came from an ancient d42ec126
> ("disable post-checkout test on Cygwin", 2009-03-17) that says
>
> disable post-checkout test on Cygwin
>
> It is broken because of the tricks we have to play with
> lstat to get the bearable perfomance out of the call.
> Sadly, it disables access to Cygwin's executable attribute,
> which Windows filesystems do not have at all.
>
> I wonder if this is still relevant these days (Cc'ed Ramsay for
> input). Windows port should be running enabled hooks (and X_OK is
> made pretty much no-op in compat/mingw.c IIUC), so the above
> conditional is overly broad anyway, even if Cygwin still has issues
> with the executable bit.
Reverted.
Thanks,
- Orgad
^ permalink raw reply
* [PATCH 1/2] t5403: simplify by using a single repository
From: orgads @ 2018-12-29 21:37 UTC (permalink / raw)
To: git; +Cc: Orgad Shaneh
From: Orgad Shaneh <orgads@gmail.com>
There is no strong reason to use separate clones to run
these tests; just use a single test repository prepared
with more modern test_commit shell helper function.
While at it, replace three "awk '{print $N}'" on the same
file with shell built-in "read" into three variables.
Revert d42ec126aa717d00549e387d5a95fd55683c2e2c which is a workaround for
Cygwin that is no longer needed.
Signed-off-by: Orgad Shaneh <orgads@gmail.com>
---
t/t5403-post-checkout-hook.sh | 78 +++++++++++------------------------
1 file changed, 23 insertions(+), 55 deletions(-)
diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
index fc898c9eac..1d15a1031f 100755
--- a/t/t5403-post-checkout-hook.sh
+++ b/t/t5403-post-checkout-hook.sh
@@ -7,82 +7,50 @@ test_description='Test the post-checkout hook.'
. ./test-lib.sh
test_expect_success setup '
- echo Data for commit0. >a &&
- echo Data for commit0. >b &&
- git update-index --add a &&
- git update-index --add b &&
- tree0=$(git write-tree) &&
- commit0=$(echo setup | git commit-tree $tree0) &&
- git update-ref refs/heads/master $commit0 &&
- git clone ./. clone1 &&
- git clone ./. clone2 &&
- GIT_DIR=clone2/.git git branch new2 &&
- echo Data for commit1. >clone2/b &&
- GIT_DIR=clone2/.git git add clone2/b &&
- GIT_DIR=clone2/.git git commit -m new2
-'
-
-for clone in 1 2; do
- cat >clone${clone}/.git/hooks/post-checkout <<'EOF'
-#!/bin/sh
-echo $@ > $GIT_DIR/post-checkout.args
-EOF
- chmod u+x clone${clone}/.git/hooks/post-checkout
-done
-
-test_expect_success 'post-checkout runs as expected ' '
- GIT_DIR=clone1/.git git checkout master &&
- test -e clone1/.git/post-checkout.args
+ mkdir -p .git/hooks &&
+ write_script .git/hooks/post-checkout <<-\EOF &&
+ echo "$@" >.git/post-checkout.args
+ EOF
+ test_commit one &&
+ test_commit two &&
+ test_commit three
'
test_expect_success 'post-checkout receives the right arguments with HEAD unchanged ' '
- old=$(awk "{print \$1}" clone1/.git/post-checkout.args) &&
- new=$(awk "{print \$2}" clone1/.git/post-checkout.args) &&
- flag=$(awk "{print \$3}" clone1/.git/post-checkout.args) &&
+ test_when_finished "rm -f .git/post-checkout.args" &&
+ git checkout master &&
+ read old new flag <.git/post-checkout.args &&
test $old = $new && test $flag = 1
'
-test_expect_success 'post-checkout runs as expected ' '
- GIT_DIR=clone1/.git git checkout master &&
- test -e clone1/.git/post-checkout.args
-'
-
test_expect_success 'post-checkout args are correct with git checkout -b ' '
- GIT_DIR=clone1/.git git checkout -b new1 &&
- old=$(awk "{print \$1}" clone1/.git/post-checkout.args) &&
- new=$(awk "{print \$2}" clone1/.git/post-checkout.args) &&
- flag=$(awk "{print \$3}" clone1/.git/post-checkout.args) &&
+ test_when_finished "rm -f .git/post-checkout.args" &&
+ git checkout -b new1 &&
+ read old new flag <.git/post-checkout.args &&
test $old = $new && test $flag = 1
'
test_expect_success 'post-checkout receives the right args with HEAD changed ' '
- GIT_DIR=clone2/.git git checkout new2 &&
- old=$(awk "{print \$1}" clone2/.git/post-checkout.args) &&
- new=$(awk "{print \$2}" clone2/.git/post-checkout.args) &&
- flag=$(awk "{print \$3}" clone2/.git/post-checkout.args) &&
+ test_when_finished "rm -f .git/post-checkout.args" &&
+ git checkout two &&
+ read old new flag <.git/post-checkout.args &&
test $old != $new && test $flag = 1
'
test_expect_success 'post-checkout receives the right args when not switching branches ' '
- GIT_DIR=clone2/.git git checkout master b &&
- old=$(awk "{print \$1}" clone2/.git/post-checkout.args) &&
- new=$(awk "{print \$2}" clone2/.git/post-checkout.args) &&
- flag=$(awk "{print \$3}" clone2/.git/post-checkout.args) &&
+ test_when_finished "rm -f .git/post-checkout.args" &&
+ git checkout master -- three.t &&
+ read old new flag <.git/post-checkout.args &&
test $old = $new && test $flag = 0
'
-if test "$(git config --bool core.filemode)" = true; then
-mkdir -p templates/hooks
-cat >templates/hooks/post-checkout <<'EOF'
-#!/bin/sh
-echo $@ > $GIT_DIR/post-checkout.args
-EOF
-chmod +x templates/hooks/post-checkout
-
test_expect_success 'post-checkout hook is triggered by clone' '
+ mkdir -p templates/hooks &&
+ write_script templates/hooks/post-checkout <<-\EOF &&
+ echo "$@" >$GIT_DIR/post-checkout.args
+ EOF
git clone --template=templates . clone3 &&
test -f clone3/.git/post-checkout.args
'
-fi
test_done
--
2.20.1
^ permalink raw reply related
* [PATCH 2/2] Rebase: Run post-checkout hook on checkout
From: orgads @ 2018-12-29 21:37 UTC (permalink / raw)
To: git; +Cc: Orgad Shaneh
In-Reply-To: <20181229213759.12878-1-orgads@gmail.com>
From: Orgad Shaneh <orgads@gmail.com>
The scripted version of rebase used to run this hook on the initial
checkout. The transition to built-in introduced a regression.
Signed-off-by: Orgad Shaneh <orgads@gmail.com>
---
builtin/rebase.c | 12 ++++++++++--
t/t5403-post-checkout-hook.sh | 20 ++++++++++++++++++++
2 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index b5c99ec10c..8402765a79 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -530,6 +530,7 @@ static int run_specific_rebase(struct rebase_options *opts)
#define RESET_HEAD_DETACH (1<<0)
#define RESET_HEAD_HARD (1<<1)
+#define RESET_HEAD_RUN_POST_CHECKOUT_HOOK (1<<2)
static int reset_head(struct object_id *oid, const char *action,
const char *switch_to_branch, unsigned flags,
@@ -537,6 +538,7 @@ static int reset_head(struct object_id *oid, const char *action,
{
unsigned detach_head = flags & RESET_HEAD_DETACH;
unsigned reset_hard = flags & RESET_HEAD_HARD;
+ unsigned run_hook = flags & RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
struct object_id head_oid;
struct tree_desc desc[2] = { { NULL }, { NULL } };
struct lock_file lock = LOCK_INIT;
@@ -636,6 +638,10 @@ static int reset_head(struct object_id *oid, const char *action,
ret = update_ref(reflog_head, "HEAD", oid, NULL, 0,
UPDATE_REFS_MSG_ON_ERR);
}
+ if (run_hook)
+ run_hook_le(NULL, "post-checkout",
+ oid_to_hex(orig ? orig : &null_oid),
+ oid_to_hex(oid), "1", NULL);
leave_reset_head:
strbuf_release(&msg);
@@ -1465,7 +1471,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
getenv(GIT_REFLOG_ACTION_ENVIRONMENT),
options.switch_to);
if (reset_head(&oid, "checkout",
- options.head_name, 0,
+ options.head_name,
+ RESET_HEAD_RUN_POST_CHECKOUT_HOOK,
NULL, buf.buf) < 0) {
ret = !!error(_("could not switch to "
"%s"),
@@ -1539,7 +1546,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
strbuf_addf(&msg, "%s: checkout %s",
getenv(GIT_REFLOG_ACTION_ENVIRONMENT), options.onto_name);
if (reset_head(&options.onto->object.oid, "checkout", NULL,
- RESET_HEAD_DETACH, NULL, msg.buf))
+ RESET_HEAD_DETACH | RESET_HEAD_RUN_POST_CHECKOUT_HOOK,
+ NULL, msg.buf))
die(_("Could not detach HEAD"));
strbuf_release(&msg);
diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
index 1d15a1031f..a539ffc080 100755
--- a/t/t5403-post-checkout-hook.sh
+++ b/t/t5403-post-checkout-hook.sh
@@ -13,6 +13,8 @@ test_expect_success setup '
EOF
test_commit one &&
test_commit two &&
+ test_commit rebase-on-me &&
+ git reset --hard HEAD^ &&
test_commit three
'
@@ -44,6 +46,24 @@ test_expect_success 'post-checkout receives the right args when not switching br
test $old = $new && test $flag = 0
'
+test_expect_success 'post-checkout is triggered on rebase' '
+ test_when_finished "rm -f .git/post-checkout.args" &&
+ git checkout -b rebase-test master &&
+ rm -f .git/post-checkout.args &&
+ git rebase rebase-on-me &&
+ read old new flag <.git/post-checkout.args &&
+ test $old != $new && test $flag = 1
+'
+
+test_expect_success 'post-checkout is triggered on rebase with fast-forward' '
+ test_when_finished "rm -f .git/post-checkout.args" &&
+ git checkout -b ff-rebase-test rebase-on-me^ &&
+ rm -f .git/post-checkout.args &&
+ git rebase rebase-on-me &&
+ read old new flag <.git/post-checkout.args &&
+ test $old != $new && test $flag = 1
+'
+
test_expect_success 'post-checkout hook is triggered by clone' '
mkdir -p templates/hooks &&
write_script templates/hooks/post-checkout <<-\EOF &&
--
2.20.1
^ permalink raw reply related
* Lack of debug info from git commit -vvv -S -am "..."
From: Jeffrey Walton @ 2018-12-29 22:24 UTC (permalink / raw)
To: Git List
I'm trying to determine why a new installation of GnuPG is having
trouble. The new install is in /usr/local. The other install is from
the distro and is OK.
Here's the new installation error message:
$ git commit -vvv -S -am "Update GnuPG recipe"
error: gpg failed to sign the data
fatal: failed to write commit object
How can I obtain more information from git commit?
^ permalink raw reply
* Git Test Coverage Report (Saturday, Dec 29)
From: Derrick Stolee @ 2018-12-29 22:31 UTC (permalink / raw)
To: git@vger.kernel.org
Here is today's test coverage report.
Thanks,
-Stolee
[1] https://dev.azure.com/git/git/_build/results?buildId=289
---
pu: e31bc98f4bb8c7cf2a943a3b3b3de69a34a4349c
jch: 5442582aa4fe91238d2c294660d08fc1e0efc8b7
next: 81188d93c3fce477216ba905bd37ab453a74b11d
master: b21ebb671bb7dea8d342225f0d66c41f4e54d5ca
master@{1}: 7a95a1cd084cb665c5c2586a415e42df0213af74
Uncovered code in 'pu' not in 'jch'
--------------------------------------
apply.c
1fcfdf84ce 4275) oidclr(&patch->old_oid);
backup-log.c
fdbbdf809f 25) return error_errno(_("unable to open %s"), path);
fdbbdf809f 27) close(fd);
fdbbdf809f 28) return error_errno(_("unable to update %s"), path);
fdbbdf809f 41) return -1;
102b7856e3 63) return -1;/* corrupt? */
102b7856e3 71) return -1; /* corrupt? */
102b7856e3 75) message += 6;
102b7856e3 105) if (errno == ENOENT || errno == ENOTDIR)
102b7856e3 106) return 0;
102b7856e3 107) return -1;
102b7856e3 112) ret = error_errno(_("cannot seek back in %s"), path);
102b7856e3 123) ret = error_errno(_("cannot seek back in %s"), path);
102b7856e3 124) break;
102b7856e3 128) ret = error_errno(_("cannot read %d bytes from %s"),
102b7856e3 130) break;
102b7856e3 189) strbuf_splice(&sb, 0, 0, buf, endp - buf);
102b7856e3 190) break;
102b7856e3 215) return -1;
102b7856e3 219) ret = parse(&sb, data);
bde028c667 232) static int good_oid(struct repository *r, const struct
object_id *oid)
bde028c667 234) if (is_null_oid(oid))
bde028c667 235) return 1;
bde028c667 237) return oid_object_info(r, oid, NULL) == OBJ_BLOB;
bde028c667 240) static int prune_parse(struct strbuf *line, void *data)
bde028c667 242) struct prune_options *opts = data;
bde028c667 245) strbuf_reset(&opts->copy);
bde028c667 246) strbuf_addbuf(&opts->copy, line);
bde028c667 248) if (bkl_parse_entry(line, &entry))
bde028c667 249) return -1;
bde028c667 251) if (entry.timestamp < opts->expire)
bde028c667 252) return 0;
bde028c667 254) if (oideq(&entry.old_oid, &entry.new_oid))
bde028c667 255) return 0;
bde028c667 257) if (!good_oid(opts->repo, &entry.old_oid) ||
bde028c667 258) !good_oid(opts->repo, &entry.new_oid))
bde028c667 259) return 0;
bde028c667 261) if (!opts->fp)
bde028c667 262) return -1;
bde028c667 264) fputs(opts->copy.buf, opts->fp);
bde028c667 265) return 0;
bde028c667 278) return error(_("failed to lock '%s'"), path);
bde028c667 287) rollback_lock_file(&lk);
b86e9ac723 301) die(_("failed to prune %s"), "gitdir.bkl");
b86e9ac723 309) if (wt->id)
b86e9ac723 310) die(_("failed to prune %s on working tree '%s'"),
b86e9ac723 313) die(_("failed to prune %s"), "index.bkl");
b86e9ac723 316) if (wt->id)
b86e9ac723 317) die(_("failed to prune %s on working tree '%s'"),
b86e9ac723 320) die(_("failed to prune %s"), "worktree.bkl");
b2069b6eb0 331) static void add_blob_to_pending(const struct object_id *oid,
b2069b6eb0 337) if (!good_oid(cb->revs->repo, oid))
b2069b6eb0 338) return;
b2069b6eb0 340) blob = lookup_blob(cb->revs->repo, oid);
b2069b6eb0 341) blob->object.flags |= cb->flags;
b2069b6eb0 342) add_pending_object(cb->revs, &blob->object, path);
b2069b6eb0 345) static int add_pending(struct strbuf *line, void *cb)
b2069b6eb0 349) if (bkl_parse_entry(line, &entry))
b2069b6eb0 350) return -1;
b2069b6eb0 352) add_blob_to_pending(&entry.old_oid, entry.path, cb);
b2069b6eb0 353) add_blob_to_pending(&entry.new_oid, entry.path, cb);
b2069b6eb0 354) return 0;
bisect.c
04dac00473 661) mark_edges_uninteresting(revs, NULL, 0);
builtin/backup-log.c
fdbbdf809f 28) usage_with_options(backup_log_usage, NULL);
fdbbdf809f 33) die(_("not a valid object name: %s"), argv[2]);
fdbbdf809f 36) die(_("not a valid object name: %s"), argv[3]);
6a05b9ab74 62) return -1;
6a05b9ab74 65) return 2;
6a05b9ab74 69) return 0;
6a05b9ab74 82) return 1;/* treat null oid like empty blob */
6a05b9ab74 86) die(_("object not found: %s"), oid_to_hex(oid));
6a05b9ab74 88) die(_("not a blob: %s"), oid_to_hex(oid));
6a05b9ab74 111) usage_with_options(backup_log_usage, options);
6a05b9ab74 114) die(_("not a valid change id: %s"), argv[0]);
6a05b9ab74 119) die(_("failed to parse '%s'"), log_path);
45f3e0cd9d 130) old = the_hash_algo->empty_blob;
45f3e0cd9d 135) new = the_hash_algo->empty_blob;
45f3e0cd9d 140) return;
45f3e0cd9d 156) return -1;
45f3e0cd9d 162) return 0;
45f3e0cd9d 183) found_dash_dash = 1;
45f3e0cd9d 184) i++;
45f3e0cd9d 185) continue;
45f3e0cd9d 189) exit(128);
45f3e0cd9d 196) die(_("not a valid change id: %s"), arg);
45f3e0cd9d 204) usage_with_options(backup_log_usage, NULL);
45f3e0cd9d 208) die(_("failed to parse '%s'"), log_path);
7f1d166ee1 252) return -1;
7f1d166ee1 255) return 1;
7f1d166ee1 260) return 0;
7f1d166ee1 283) opts.revs.diffopt.output_format = DIFF_FORMAT_PATCH;
7f1d166ee1 284) diff_setup_done(&opts.revs.diffopt);
7f1d166ee1 293) ret = bkl_parse_file(log_path, log_parse, &opts);
7f1d166ee1 298) die(_("failed to parse '%s'"), log_path);
bde028c667 304) static int prune(int argc, const char **argv,
bde028c667 307) timestamp_t expire = time(NULL) - 90 * 24 * 3600;
bde028c667 308) struct option options[] = {
bde028c667 314) argc = parse_options(argc, argv, prefix, options,
backup_log_usage, 0);
bde028c667 316) return bkl_prune(the_repository, log_path, expire);
fdbbdf809f 323) else if (!strcmp(id, "worktree"))
fdbbdf809f 324) return git_pathdup("worktree.bkl");
fdbbdf809f 325) else if (!strcmp(id, "gitdir"))
fdbbdf809f 326) return git_pathdup("common/gitdir.bkl");
fdbbdf809f 328) die(_("backup log id '%s' is not recognized"), id);
fdbbdf809f 346) die(_("expected a subcommand"));
fdbbdf809f 350) die(_("--id and --path are incompatible"));
fdbbdf809f 354) die(_("either --id or --path is required"));
bde028c667 364) else if (!strcmp(argv[0], "prune"))
bde028c667 365) return prune(argc, argv, prefix, log_path);
builtin/blame.c
74e8221b52 builtin/blame.c 930) blame_date_width = sizeof("Thu Oct 19
16:00");
74e8221b52 builtin/blame.c 931) break;
builtin/checkout.c
20e835df5c builtin/checkout.c 1271) die(_("'%s' cannot be used with
switching branches"),
4ed0c8eb49 builtin/checkout.c 1275) die(_("'%s' cannot be used with
switching branches"),
builtin/config.c
937d6bee9e builtin/config.c 604) oidclr(oid);
builtin/multi-pack-index.c
e86a2be882 49) die(_("--batch-size option is only for 'repack' verb"));
builtin/rebase.c
81ef8ee75d 764) return -1;
d421afa0c6 1253) die(_("--reschedule-failed-exec requires an interactive
rebase"));
d421afa0c6 1291) die(_("error: cannot combine '--preserve-merges' with "
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(&now, NULL);
74e8221b52 232) show_date_relative(time, tz, &now, 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(&now, NULL);
74e8221b52 290) human_tz = local_time_tzoffset(now.tv_sec, &human_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();
entry.c
list-objects.c
04dac00473 241) continue;
04dac00473 250) parent->object.flags |= SHOWN;
04dac00473 251) show_edge(parent);
04dac00473 274) tree->object.flags |= UNINTERESTING;
midx.c
5518ac8ee4 806) error(_("did not see pack-file %s to drop"),
5518ac8ee4 808) drop_index++;
5518ac8ee4 809) i--;
5518ac8ee4 810) missing_drops++;
5518ac8ee4 826) error(_("did not see all pack-files to drop"));
5518ac8ee4 827) result = 1;
5518ac8ee4 828) goto cleanup;
5518ac8ee4 1078) return 0;
5518ac8ee4 1095) close_pack(m->packs[i]);
5518ac8ee4 1096) m->packs[i] = NULL;
b59c492264 1133) return -1;
b59c492264 1135) return 1;
b59c492264 1151) return 0;
b59c492264 1160) continue;
b59c492264 1173) continue;
b59c492264 1196) error(_("could not start pack-objects"));
b59c492264 1197) result = 1;
b59c492264 1198) goto cleanup;
b59c492264 1215) error(_("could not finish pack-objects"));
b59c492264 1216) result = 1;
b59c492264 1217) goto cleanup;
pretty.c
4681fe38e1 1069) return 0;
b755bf6f83 1107) match_placeholder_arg(p, "=on", end) ||
b755bf6f83 1108) match_placeholder_arg(p, "=true", end)) {
protocol.c
24c10f7473 37) die(_("Unrecognized protocol version"));
24c10f7473 39) die(_("Unrecognized protocol_version"));
read-cache.c
43bf1db73e 1323) oidcpy(&backup_prev, &istate->cache[pos]->oid);
43bf1db73e 1343) update_backup_log(istate, &backup_prev, ce);
43bf1db73e 3215) strbuf_release(&sb);
43bf1db73e 3216) return -1;
refs/files-backend.c
c67027c9a9 1892) return;
c67027c9a9 1895) return;
remote-curl.c
24c10f7473 400) return 0;
revision.c
497f2693ab 149) return;
497f2693ab 152) return;
497f2693ab 175) break;
04dac00473 197) continue;
strbuf.c
18f8e81091 397) return 0;
submodule.c
26f80ccfc1 1396) strbuf_release(&gitdir);
be76c21282 1519) struct fetch_task *task = task_cb;
be76c21282 1523) fetch_task_release(task);
unpack-trees.c
cc14089d7c 206) oidclr(&null_hash);
cc14089d7c 207) new_hash = &null_hash;
6f41cc899b 1716) index_path(NULL, old_hash, ce->name, &st,
6f41cc899b 1972) old_hash && !lstat(ce->name, &st))
6f41cc899b 1973) index_path(NULL, old_hash, ce->name, &st,
cc14089d7c 2291) if (verify_absent(ce, ERROR_WOULD_LOSE_UNTRACKED_REMOVED,
cc14089d7c 2294) make_backup(ce, &old_hash, NULL, o);
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:
Anders Waldenborg 18f8e8109: strbuf: separate callback for
strbuf_expand:ing literals
Anders Waldenborg 4681fe38e: pretty: allow showing specific trailers
Anders Waldenborg b755bf6f8: pretty: allow %(trailers) options with
explicit value
Derrick Stolee 04dac0047: list-objects: consume sparse tree walk
Derrick Stolee 497f2693a: revision: implement sparse algorithm
Derrick Stolee 5518ac8ee: multi-pack-index: implement 'expire' verb
Derrick Stolee b59c49226: multi-pack-index: implement midx_repack()
Derrick Stolee e86a2be88: multi-pack-index: prepare 'repack' verb
Johannes Schindelin 81ef8ee75: rebase: introduce a shortcut for
--reschedule-failed-exec
Johannes Schindelin d421afa0c: rebase: introduce
--reschedule-failed-exec
Josh Steadmon 24c10f747: protocol: advertise multiple supported
versions
Linus Torvalds 74e8221b5: Add 'human' date format
Martin Koegler 5efde212f: zlib.c: use size_t for size
Nguyễn Thái Ngọc Duy 102b7856e: backup-log.c: add API for walking
backup log
Nguyễn Thái Ngọc Duy 1fcfdf84c: apply: support backup log with
--keep-backup
Nguyễn Thái Ngọc Duy 43bf1db73: read-cache.c: new flag for
add_index_entry() to write to backup log
Nguyễn Thái Ngọc Duy 45f3e0cd9: backup-log: add diff command
Nguyễn Thái Ngọc Duy 6a05b9ab7: backup-log: add cat command
Nguyễn Thái Ngọc Duy 6f41cc899: reset --hard: keep backup of
overwritten files
Nguyễn Thái Ngọc Duy 7f1d166ee: backup-log: add log command
Nguyễn Thái Ngọc Duy 937d6bee9: config --edit: support backup log
Nguyễn Thái Ngọc Duy b2069b6eb: backup-log: keep all blob
references around
Nguyễn Thái Ngọc Duy b86e9ac72: gc: prune backup logs
Nguyễn Thái Ngọc Duy bde028c66: backup-log: add prune command
Nguyễn Thái Ngọc Duy c67027c9a: refs: keep backup of deleted reflog
Nguyễn Thái Ngọc Duy cc14089d7: unpack-trees.c: keep backup of
ignored files being overwritten
Nguyễn Thái Ngọc Duy fdbbdf809: backup-log: add "update" subcommand
Stefan Beller 26f80ccfc: submodule: migrate get_next_submodule to
use repository structs
Stefan Beller be76c2128: fetch: ensure submodule objects fetched
Thomas Gummerer 20e835df5: checkout: add --cached option
Thomas Gummerer 4ed0c8eb4: checkout: introduce --{,no-}overlay option
Uncovered code in 'jch' not in 'next'
----------------------------------------
apply.c
0f086e6dca 3355) if (checkout_entry(ce, &costate, NULL, NULL) ||
0f086e6dca 3356) lstat(ce->name, st))
builtin/branch.c
0ecb1fc726 builtin/branch.c 456) die(_("could not resolve HEAD"));
0ecb1fc726 builtin/branch.c 462) die(_("HEAD (%s) points outside of
refs/heads/"), refname);
builtin/pull.c
b19eee9066 647) argv_array_push(&args, opt_cleanup);
builtin/remote.c
f39a9c6547 builtin/remote.c 1551) die(_("--save-to-push cannot be used
with other options"));
f39a9c6547 builtin/remote.c 1575) die(_("--save-to-push can only be used
when only one url is defined"));
commit-graph.c
721351787e 128) return NULL;
721351787e 131) return NULL;
721351787e 187) free(graph);
721351787e 188) return NULL;
721351787e 223) free(graph);
721351787e 224) return NULL;
hex.c
47edb64997 93) char *sha1_to_hex_r(char *buffer, const unsigned char *sha1)
47edb64997 95) return hash_to_hex_algop_r(buffer, sha1,
&hash_algos[GIT_HASH_SHA1]);
47edb64997 116) char *hash_to_hex(const unsigned char *hash)
47edb64997 118) return hash_to_hex_algop(hash, the_hash_algo);
pathspec.c
22af33bece 671) name = to_free = xmemdupz(name, namelen);
read-cache.c
ee70c12820 1730) if (advice_unknown_index_extension) {
ee70c12820 1731) warning(_("ignoring optional %.4s index extension"), ext);
ee70c12820 1732) advise(_("This is likely due to the file having been
written by a newer\n"
ec36c42a63 3508) const char *index = NULL;
ec36c42a63 3514) if (!offset)
ec36c42a63 3515) return NULL;
ec36c42a63 3516) while (offset <= mmap_size - the_hash_algo->rawsz - 8) {
ec36c42a63 3517) extsize = get_be32(mmap + offset + 4);
ec36c42a63 3518) if (CACHE_EXT((mmap + offset)) ==
CACHE_EXT_INDEXENTRYOFFSETTABLE) {
ec36c42a63 3519) index = mmap + offset + 4 + 4;
ec36c42a63 3520) break;
ec36c42a63 3522) offset += 8;
ec36c42a63 3523) offset += extsize;
ec36c42a63 3525) if (!index)
ec36c42a63 3526) return NULL;
ec36c42a63 3529) ext_version = get_be32(index);
ec36c42a63 3530) if (ext_version != IEOT_VERSION) {
ec36c42a63 3531) error("invalid IEOT version %d", ext_version);
ec36c42a63 3532) return NULL;
ec36c42a63 3534) index += sizeof(uint32_t);
ec36c42a63 3537) nr = (extsize - sizeof(uint32_t)) / (sizeof(uint32_t) +
sizeof(uint32_t));
ec36c42a63 3538) if (!nr) {
ec36c42a63 3539) error("invalid number of IEOT entries %d", nr);
ec36c42a63 3540) return NULL;
ec36c42a63 3542) ieot = xmalloc(sizeof(struct index_entry_offset_table)
ec36c42a63 3543) + (nr * sizeof(struct index_entry_offset)));
ec36c42a63 3544) ieot->nr = nr;
ec36c42a63 3545) for (i = 0; i < nr; i++) {
ec36c42a63 3546) ieot->entries[i].offset = get_be32(index);
ec36c42a63 3547) index += sizeof(uint32_t);
ec36c42a63 3548) ieot->entries[i].nr = get_be32(index);
ec36c42a63 3549) index += sizeof(uint32_t);
ec36c42a63 3552) return ieot;
ref-filter.c
bbfc042ef9 238) oi_deref.info.sizep = &oi_deref.size;
bbfc042ef9 247) return strbuf_addf_ret(err, -1, _("unrecognized
%%(objectsize) argument: %s"), arg);
ab0e367154 255) return strbuf_addf_ret(err, -1, _("%%(deltabase) does
not take arguments"));
remote-curl.c
80bb63786c 359) die("invalid server response; expected service, got
flush packet");
sequencer.c
18e711a162 2389) opts->quiet = 1;
sha1-file.c
2f90b9d9b4 sha1-file.c 172) int hash_algo_by_name(const char *name)
2f90b9d9b4 sha1-file.c 175) if (!name)
2f90b9d9b4 sha1-file.c 176) return GIT_HASH_UNKNOWN;
2f90b9d9b4 sha1-file.c 177) for (i = 1; i < GIT_HASH_NALGOS; i++)
2f90b9d9b4 sha1-file.c 178) if (!strcmp(name, hash_algos[i].name))
2f90b9d9b4 sha1-file.c 179) return i;
2f90b9d9b4 sha1-file.c 180) return GIT_HASH_UNKNOWN;
2f90b9d9b4 sha1-file.c 183) int hash_algo_by_id(uint32_t format_id)
2f90b9d9b4 sha1-file.c 186) for (i = 1; i < GIT_HASH_NALGOS; i++)
2f90b9d9b4 sha1-file.c 187) if (format_id == hash_algos[i].format_id)
2f90b9d9b4 sha1-file.c 188) return i;
2f90b9d9b4 sha1-file.c 189) return GIT_HASH_UNKNOWN;
transport-helper.c
3b3357626e 1029) static int has_attribute(const char *attrs, const char
*attr)
tree.c
e092073d64 104) commit = lookup_commit(r, entry.oid);
Commits introducing uncovered code:
brian m. carlson 2f90b9d9b: sha1-file: provide functions to look up
hash algorithms
brian m. carlson 47edb6499: hex: introduce functions to print
arbitrary hashes
Daniels Umanovskis 0ecb1fc72: branch: introduce --show-current
display option
Denton Liu b19eee906: merge: add scissors line on merge conflict
Denton Liu f39a9c654: remote: add --save-to-push option to git
remote set-url
Elijah Newren 18e711a16: git-rebase, sequencer: extend --quiet
option for the interactive machinery
Jeff King 80bb63786: remote-curl: refactor smart-http discovery
Jonathan Nieder ee70c1282: index: offer advice for unknown index
extensions
Josh Steadmon 721351787: commit-graph, fuzz: add fuzzer for
commit-graph
Nguyễn Thái Ngọc Duy 0f086e6dc: checkout: print something when
checking out paths
Nguyễn Thái Ngọc Duy 22af33bec: dir.c: move, rename and export
match_attrs()
Nguyễn Thái Ngọc Duy 3b3357626: style: the opening '{' of a
function is in a separate line
Nguyễn Thái Ngọc Duy e092073d6: tree.c: make read_tree*() take
'struct repository *'
Nguyễn Thái Ngọc Duy ec36c42a6: Indent code with TABs
Olga Telezhnaya ab0e36715: ref-filter: add deltabase option
Olga Telezhnaya bbfc042ef: ref-filter: add objectsize:disk option
Uncovered code in 'next' not in 'master'
--------------------------------------------
archive.c
c6e7965ddf 399) die(_("not a valid object name: %s"), name);
c6e7965ddf 412) die(_("not a tree object: %s"), oid_to_hex(&oid));
c6e7965ddf 422) die(_("current working directory is untracked"));
attr.c
ad8f8f4aed 369) fprintf_ln(stderr, _("%s not allowed: %s:%d"),
blame.c
fb998eae6c 1717) obj = deref_tag(revs->repo, obj, NULL, 0);
fb998eae6c 1724) head_commit = lookup_commit_reference_gently(revs->repo,
builtin/bundle.c
74ae4b638d builtin/bundle.c 64) return !!unbundle(the_repository,
&header, bundle_fd, 0) ||
builtin/fast-export.c
b93b81e799 builtin/fast-export.c 52) signed_tag_mode = SIGNED_TAG_ABORT;
b93b81e799 builtin/fast-export.c 70) tag_of_filtered_mode =
TAG_FILTERING_ABORT;
f129c4275c builtin/fast-export.c 202) if (!p->parents)
f129c4275c builtin/fast-export.c 203) return NULL;
f129c4275c builtin/fast-export.c 204) p = p->parents->item;
f129c4275c builtin/fast-export.c 205) }
843b9e6d48 builtin/fast-export.c 265) die("oid mismatch in blob %s",
oid_to_hex(oid));
a965bb3116 builtin/fast-export.c 277) printf("original-oid %s\n",
oid_to_hex(oid));
843b9e6d48 builtin/fast-export.c 356) const unsigned hashsz =
the_hash_algo->rawsz;
843b9e6d48 builtin/fast-export.c 357) unsigned char *out =
xcalloc(hashsz, 1);
843b9e6d48 builtin/fast-export.c 358) put_be32(out + hashsz - 4,
counter++);
843b9e6d48 builtin/fast-export.c 362) static const struct object_id
*anonymize_oid(const struct object_id *oid)
843b9e6d48 builtin/fast-export.c 365) size_t len = the_hash_algo->rawsz;
843b9e6d48 builtin/fast-export.c 366) return anonymize_mem(&objs,
generate_fake_oid, oid, &len);
843b9e6d48 builtin/fast-export.c 426) anonymize_oid(&spec->oid) :
a965bb3116 builtin/fast-export.c 644) printf("original-oid %s\n",
oid_to_hex(&commit->object.oid));
530ca19c02 builtin/fast-export.c 668) printf("%s\n", oid_to_hex(anonymize ?
530ca19c02 builtin/fast-export.c 669) anonymize_oid(&obj->oid) :
f129c4275c builtin/fast-export.c 810) p = rewrite_commit((struct commit
*)tagged);
f129c4275c builtin/fast-export.c 811) if (!p) {
f129c4275c builtin/fast-export.c 812) printf("reset %s\nfrom %s\n\n",
f129c4275c builtin/fast-export.c 814) free(buf);
f129c4275c builtin/fast-export.c 815) return;
a965bb3116 builtin/fast-export.c 825) printf("original-oid %s\n",
oid_to_hex(&tag->object.oid));
cd13762d8f builtin/fast-export.c 943) printf("reset %s\nfrom %s\n\n",
cd13762d8f builtin/fast-export.c 945) continue;
530ca19c02 builtin/fast-export.c 960) if (!reference_excluded_commits) {
530ca19c02 builtin/fast-export.c 962) printf("reset %s\nfrom %s\n\n",
530ca19c02 builtin/fast-export.c 964) continue;
530ca19c02 builtin/fast-export.c 967) printf("reset %s\nfrom %s\n\n", name,
530ca19c02 builtin/fast-export.c 968) oid_to_hex(&commit->object.oid));
fdf31b6369 builtin/fast-export.c 969) continue;
builtin/fsck.c
674ba34038 builtin/fsck.c 87) ret = _("unknown");
674ba34038 builtin/fsck.c 167) objerror(parent, _("wrong object type in
link"));
674ba34038 builtin/fsck.c 278) printf_ln(_("unreachable %s %s"),
printable_type(obj),
674ba34038 builtin/fsck.c 306) error(_("could not create lost-found"));
674ba34038 builtin/fsck.c 313) die_errno(_("could not write '%s'"),
filename);
674ba34038 builtin/fsck.c 317) die_errno(_("could not finish '%s'"),
674ba34038 builtin/fsck.c 334) fprintf_ln(stderr, _("Checking %s"),
describe_object(obj));
674ba34038 builtin/fsck.c 352) fprintf_ln(stderr, _("Checking
connectivity (%d objects)"), max);
674ba34038 builtin/fsck.c 371) fprintf_ln(stderr, _("Checking %s %s"),
674ba34038 builtin/fsck.c 384) printf_ln(_("root %s"),
674ba34038 builtin/fsck.c 420) return error(_("%s: object corrupt or
missing"),
674ba34038 builtin/fsck.c 459) fprintf_ln(stderr, _("Checking reflog
%s->%s"),
674ba34038 builtin/fsck.c 583) error(_("%s: object could not be parsed:
%s"),
674ba34038 builtin/fsck.c 618) fprintf_ln(stderr, _("Checking object
directory"));
8e2de8338e builtin/fsck.c 636) fprintf_ln(stderr, _("Checking %s link"),
head_ref_name);
8e2de8338e builtin/fsck.c 641) return error(_("invalid %s"), head_ref_name);
674ba34038 builtin/fsck.c 670) fprintf_ln(stderr, _("Checking cache tree"));
674ba34038 builtin/fsck.c 686) err |= objerror(obj, _("non-tree in
cache-tree"));
builtin/merge.c
9440b831ad builtin/merge.c 131) return error(_("option `%s' requires a
value"), opt->long_name);
builtin/rebase--interactive.c
005af339c9 builtin/rebase--interactive.c 262) ret =
rearrange_squash(the_repository);
005af339c9 builtin/rebase--interactive.c 265) ret =
sequencer_add_exec_commands(the_repository, cmd);
builtin/reflog.c
dd509db342 builtin/reflog.c 592) usage(_(reflog_expire_usage));
dd509db342 builtin/reflog.c 643) status |= error(_("%s points
nowhere!"), argv[i]);
dd509db342 builtin/reflog.c 689) usage(_(reflog_delete_usage));
dd509db342 builtin/reflog.c 695) return error(_("no reflog specified to
delete"));
dd509db342 builtin/reflog.c 704) status |= error(_("not a reflog: %s"),
argv[i]);
dd509db342 builtin/reflog.c 709) status |= error(_("no reflog for
'%s'"), argv[i]);
dd509db342 builtin/reflog.c 744) usage(_(reflog_exists_usage));
dd509db342 builtin/reflog.c 752) usage(_(reflog_exists_usage));
dd509db342 builtin/reflog.c 755) die(_("invalid ref format: %s"),
argv[start]);
builtin/repack.c
c83d950e59 200) die(_("could not start pack-objects to repack promisor
objects"));
8e2de8338e 239) die(_("repack: Expecting full hex object ID lines only
from pack-objects."));
c83d950e59 250) die_errno(_("unable to create '%s'"), promisor_name);
8e2de8338e 411) die(_("repack: Expecting full hex object ID lines only
from pack-objects."));
bundle.c
74ae4b638d 394) struct commit *one = lookup_commit_reference(revs->repo,
&oid);
delta-islands.c
385cb64ff3 216) parse_object(r, &obj->oid);
fast-import.c
a965bb3116 1821) read_next_command();
git.c
8aa8c14097 341) die_errno(_("while expanding alias '%s': '%s'"),
8aa8c14097 350) die(_("alias '%s' changes environment variables.\n"
8aa8c14097 358) die(_("empty alias for %s"), alias_command);
8aa8c14097 361) die(_("recursive alias: %s"), alias_command);
8aa8c14097 412) die(_("%s doesn't support --super-prefix"), p->cmd);
8aa8c14097 436) die_errno(_("write failure on standard output"));
8aa8c14097 438) die(_("unknown write failure on standard output"));
8aa8c14097 440) die_errno(_("close failed on standard output"));
8aa8c14097 657) die(_("%s doesn't support --super-prefix"), argv[0]);
8aa8c14097 769) die(_("cannot handle %s as a builtin"), cmd);
http-walker.c
b69fb867b4 http-walker.c 550) loose_object_path(the_repository, &buf,
req->sha1);
http.c
d73019feb4 289) return git_config_string(&curl_http_version, var, value);
d73019feb4 797) static int get_curl_http_version_opt(const char
*version_string, long *opt)
d73019feb4 808) for (i = 0; i < ARRAY_SIZE(choice); i++) {
d73019feb4 809) if (!strcmp(version_string, choice[i].name)) {
d73019feb4 810) *opt = choice[i].opt_token;
d73019feb4 811) return 0;
d73019feb4 815) warning("unknown value given to http.version: '%s'",
version_string);
d73019feb4 816) return -1; /* not found */
d73019feb4 841) if (!get_curl_http_version_opt(curl_http_version, &opt)) {
d73019feb4 843) curl_easy_setopt(result, CURLOPT_HTTP_VERSION, opt);
merge-recursive.c
37b65ce36b 1584) return -1;
37b65ce36b 1587) return -1;
37b65ce36b 1593) return -1;
37b65ce36b 1596) return -1;
37b65ce36b 1663) return -1;
37b65ce36b 1666) return -1;
37b65ce36b 1669) return -1;
7f8671656f 1702) return -1;
48c9cb9d6d 1758) return -1;
48c9cb9d6d 1806) return -1;
48c9cb9d6d 1812) return -1;
48c9cb9d6d 1815) return -1;
48c9cb9d6d 1825) return -1;
48c9cb9d6d 1831) return -1;
48c9cb9d6d 1834) return -1;
parse-options-cb.c
9440b831ad 21) return error(_("option `%s' expects a numerical value"),
9440b831ad 51) return error(_("option `%s' expects \"always\",
\"auto\", or \"never\""),
parse-options.c
9440b831ad 88) return error(_("%s takes no value"), optname(opt, flags));
9440b831ad 90) return error(_("%s isn't available"), optname(opt, flags));
9440b831ad 92) return error(_("%s takes no value"), optname(opt, flags));
9440b831ad 178) return error(_("%s expects a numerical value"),
9440b831ad 194) return error(_("%s expects a non-negative integer value"
8900342628 356) error(_("did you mean `--%s` (with two dashes ?)"), arg);
8900342628 653) error(_("unknown non-ascii option in string: `%s'"),
9440b831ad 787) strbuf_addf(&sb, "option `no-%s'", opt->long_name);
read-cache.c
9d0a9e9089 675) die(_("will not add file alias '%s' ('%s' already
exists in index)"),
9d0a9e9089 676) ce->name, alias->name);
9d0a9e9089 691) die(_("cannot create an empty blob in the object
database"));
9d0a9e9089 712) return error(_("%s: can only add regular files,
symbolic links or git-directories"), path);
9d0a9e9089 786) return error(_("unable to add '%s' to index"), path);
9d0a9e9089 822) error(_("invalid path '%s'"), path);
9d0a9e9089 848) error(_("invalid path '%s'"), path);
9d0a9e9089 1686) return error(_("bad signature 0x%08x"),
hdr->hdr_signature);
9d0a9e9089 1689) return error(_("bad index version %d"), hdr_version);
9d0a9e9089 1728) return error(_("index uses %.4s extension, which we do
not understand"),
9d0a9e9089 1730) fprintf_ln(stderr, _("ignoring %.4s extension"), ext);
9d0a9e9089 1777) die(_("unknown index entry format 0x%08x"),
extended_flags);
9d0a9e9089 1848) die(_("unordered stage entries in index"));
9d0a9e9089 1851) die(_("multiple stage entries for merged file '%s'"),
9d0a9e9089 1854) die(_("unordered stage entries for '%s'"),
9d0a9e9089 2148) die_errno(_("%s: index file open failed"), path);
9d0a9e9089 2152) die_errno(_("%s: cannot stat the open index"), path);
9d0a9e9089 2156) die(_("%s: index file smaller than expected"), path);
9d0a9e9089 2160) die_errno(_("%s: unable to map index file"), path);
9d0a9e9089 2251) warning(_("could not freshen shared index '%s'"),
shared_index);
9d0a9e9089 2286) die(_("broken index, expect %s in %s, got %s"),
9d0a9e9089 3100) error(_("cannot fix permission bits on '%s'"),
get_tempfile_path(*temp));
9d0a9e9089 3247) return error(_("%s: cannot drop to stage #0"),
ref-filter.c
9440b831ad 2330) return error(_("option `%s' is incompatible with
--no-merged"),
remote.c
0b9c3afdbf 363) warning(_("config remote shorthand cannot begin with
'/': %s"),
0b9c3afdbf 418) error(_("more than one uploadpack given, using the
first"));
0b9c3afdbf 684) die(_("key '%s' of pattern had no '*'"), key);
0b9c3afdbf 694) die(_("value '%s' of pattern has no '*'"), value);
0b9c3afdbf 1102) error(_("unable to delete '%s': remote ref does not
exist"),
0b9c3afdbf 1121) return error(_("dst ref %s receives from more than one
src"),
0b9c3afdbf 1840) die(_("couldn't find remote ref %s"), name);
0b9c3afdbf 1853) error(_("* Ignoring funny ref '%s' locally"),
0b9c3afdbf 1948) die(_("revision walk setup failed"));
0b9c3afdbf 2221) return error(_("cannot parse expected object name '%s'"),
sequencer.c
f11c958054 593) istate->cache_tree = cache_tree();
f11c958054 3974) res = error_dirty_index(r->index, opts);
sha1-file.c
f0eaf63819 sha1-file.c 2139) return r;
Commits introducing uncovered code:
Elijah Newren 37b65ce36: merge-recursive: new function for better
colliding conflict resolutions
Elijah Newren 48c9cb9d6: merge-recursive: improve
rename/rename(1to2)/add[/add] handling
Elijah Newren 530ca19c0: fast-export: add
--reference-excluded-parents option
Elijah Newren 7f8671656: merge-recursive: fix rename/add conflict
handling
Elijah Newren 843b9e6d4: fast-export: convert sha1 to oid
Elijah Newren a965bb311: fast-export: add a --show-original-ids
option to show original names
Elijah Newren b93b81e79: fast-export: use value from correct enum
Elijah Newren cd13762d8: fast-export: when using paths, avoid
corrupt stream with non-existent mark
Elijah Newren f129c4275: fast-export: move commit rewriting logic
into a function for reuse
Elijah Newren fdf31b636: fast-export: ensure we export requested refs
Force Charlie d73019feb: http: add support selecting http version
Jeff King b69fb867b: sha1_file_name(): overwrite buffer instead of
appending
Jeff King f0eaf6381: sha1-file: use an object_directory for the
main object dir
Junio C Hamano 8e2de8338: Merge branch 'nd/i18n' into next
Nguyễn Thái Ngọc Duy 005af339c: sequencer.c: remove implicit
dependency on the_repository
Nguyễn Thái Ngọc Duy 0b9c3afdb: remote.c: mark messages for translation
Nguyễn Thái Ngọc Duy 385cb64ff: delta-islands.c: remove
the_repository references
Nguyễn Thái Ngọc Duy 674ba3403: fsck: mark strings for translation
Nguyễn Thái Ngọc Duy 74ae4b638: bundle.c: remove the_repository
references
Nguyễn Thái Ngọc Duy 890034262: parse-options.c: mark more strings
for translation
Nguyễn Thái Ngọc Duy 8aa8c1409: git.c: mark more strings for
translation
Nguyễn Thái Ngọc Duy 9440b831a: parse-options: replace opterror()
with optname()
Nguyễn Thái Ngọc Duy 9d0a9e908: read-cache.c: mark more strings for
translation
Nguyễn Thái Ngọc Duy ad8f8f4ae: attr.c: mark more string for
translation
Nguyễn Thái Ngọc Duy c6e7965dd: archive.c: mark more strings for
translation
Nguyễn Thái Ngọc Duy c83d950e5: repack: mark more strings for
translation
Nguyễn Thái Ngọc Duy dd509db34: reflog: mark strings for translation
Nguyễn Thái Ngọc Duy f11c95805: sequencer.c: remove implicit
dependency on the_index
Nguyễn Thái Ngọc Duy fb998eae6: blame.c: remove implicit dependency
the_repository
^ permalink raw reply
* Re: [PATCH 0/2] Improve documentation on UTF-16
From: brian m. carlson @ 2018-12-29 23:17 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Lars Schneider, Torsten Bögershausen
In-Reply-To: <87lg4adoo5.fsf@evledraar.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 644 bytes --]
On Fri, Dec 28, 2018 at 09:46:18AM +0100, Ævar Arnfjörð Bjarmason wrote:
>
> On Thu, Dec 27 2018, brian m. carlson wrote:
>
> > We've recently fielded several reports from unhappy Windows users about
> > our handling of UTF-16, UTF-16LE, and UTF-16BE, none of which seem to be
> > suitable for certain Windows programs.
>
> Just for context, is "we" here $DAYJOB or a reference to some previous
> ML thread(s) on this list, or something else?
"We" in this case is the Git list. I think the list has seen at least
three threads in recent months.
--
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]
^ permalink raw reply
* Re: Lack of debug info from git commit -vvv -S -am "..."
From: Duy Nguyen @ 2018-12-30 5:07 UTC (permalink / raw)
To: noloader; +Cc: Git List
In-Reply-To: <CAH8yC8=zaL3ETrOSk_3xQobfG-z2VLMsGn8O-q0zCaqw6C_Bqg@mail.gmail.com>
On Sun, Dec 30, 2018 at 7:38 AM Jeffrey Walton <noloader@gmail.com> wrote:
>
> I'm trying to determine why a new installation of GnuPG is having
> trouble. The new install is in /usr/local. The other install is from
> the distro and is OK.
>
> Here's the new installation error message:
>
> $ git commit -vvv -S -am "Update GnuPG recipe"
> error: gpg failed to sign the data
> fatal: failed to write commit object
>
> How can I obtain more information from git commit?
try
$ GIT_TRACE=1 git commit ...
That should give you more info and the exact gnupg command to execute
should be there too
--
Duy
^ permalink raw reply
* [PATCH v2] zsh: complete unquoted paths with spaces correctly
From: Chayoung You @ 2018-12-30 5:31 UTC (permalink / raw)
To: git; +Cc: SZEDER Gábor, Junio C Hamano
In-Reply-To: <xmqqefa1kzcp.fsf@gitster-ct.c.googlers.com>
The following is the description of -Q flag of zsh compadd [1]:
This flag instructs the completion code not to quote any
metacharacters in the words when inserting them into the command
line.
Let's say there is a file named 'foo bar.txt' in repository, but it's
not yet added to the repository. Then the following command triggers a
completion:
git add fo<Tab>
git add 'fo<Tab>
git add "fo<Tab>
The completion results in bash:
git add foo\ bar.txt
git add 'foo bar.txt'
git add "foo bar.txt"
While them in zsh:
git add foo bar.txt
git add 'foo bar.txt'
git add "foo bar.txt"
The first one, where the pathname is not enclosed in quotes, should
escape the space with a backslash, just like bash completion does.
Otherwise, this leads git to think there are two files; foo, and
bar.txt.
The main cause of this behavior is __gitcomp_file_direct(). The both
implementions of bash and zsh are called with an argument 'foo bar.txt',
but only bash adds a backslash before a space on command line.
[1]: http://zsh.sourceforge.net/Doc/Release/Completion-Widgets.html
Signed-off-by: Chayoung You <yousbe@gmail.com>
Reviewed-by: Junio C Hamano <gitster@pobox.com>
---
contrib/completion/git-completion.bash | 4 ++--
contrib/completion/git-completion.zsh | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 9e8ec95c3..816ee3280 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2993,7 +2993,7 @@ if [[ -n ${ZSH_VERSION-} ]] &&
local IFS=$'\n'
compset -P '*[=:]'
- compadd -Q -f -- ${=1} && _ret=0
+ compadd -f -- ${=1} && _ret=0
}
__gitcomp_file ()
@@ -3002,7 +3002,7 @@ if [[ -n ${ZSH_VERSION-} ]] &&
local IFS=$'\n'
compset -P '*[=:]'
- compadd -Q -p "${2-}" -f -- ${=1} && _ret=0
+ compadd -p "${2-}" -f -- ${=1} && _ret=0
}
_git ()
diff --git a/contrib/completion/git-completion.zsh b/contrib/completion/git-completion.zsh
index 049d6b80f..886bf95d1 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -99,7 +99,7 @@ __gitcomp_file_direct ()
local IFS=$'\n'
compset -P '*[=:]'
- compadd -Q -f -- ${=1} && _ret=0
+ compadd -f -- ${=1} && _ret=0
}
__gitcomp_file ()
@@ -108,7 +108,7 @@ __gitcomp_file ()
local IFS=$'\n'
compset -P '*[=:]'
- compadd -Q -p "${2-}" -f -- ${=1} && _ret=0
+ compadd -p "${2-}" -f -- ${=1} && _ret=0
}
__git_zsh_bash_func ()
--
2.20.1
^ permalink raw reply related
* [BUG] t0410 2.20.1 fails on NonStop NSX platform
From: Randall S. Becker @ 2018-12-30 18:36 UTC (permalink / raw)
To: git
This is a bit of a head-scratcher. There are two NonStop platforms, which
behave essentially the same way. One, NSE, uses ia64, while NSX uses x64.
There are subtle differences in ksh and bash for the platforms based on what
is released when. The NSE platform has no problem running t0410 in Jenkins.
However, NSX fails in a few spots:
This is at 2.20.1. Our last port on this platform variant was 2.16.1, where
there were no unexplained issues.
I am suspecting that the problem is shell related, but I'm just not sure
what the test is supposed to do here. There are 9 other failures that appear
related, with the No such file or directory condition. If I add the -f flag
to rm in delete_object(), all t0410 tests pass except the last (22), but I
don't think that meets the intent of test itself. I wonder whether one of
the authors of the test might chime in on this.
The first, subtest 12 results in:
Initialized empty Git repository in /mypath/git/t/trash
directory.t0410-partial-clone/repo/.git/
[master (root-commit) 7745948] foo
Author: A U Thor <author@example.com>
1 file changed, 1 insertion(+)
create mode 100644 foo.t
[master e514b54] bar
Author: A U Thor <author@example.com>
1 file changed, 1 insertion(+)
create mode 100644 bar.t
Enumerating objects: 1, done.
Counting objects: 100% (1/1), done.
Writing objects: 100% (1/1), done.
Total 1 (delta 0), reused 0 (delta 0)
2b7df34bc09be010087307b898e994ce709c0db1
rm: cannot remove
'repo/.git/objects/2b/7df34bc09be010087307b898e994ce709c0db1': No such file
or directory
not ok 12 - rev-list stops traversal at missing and promised commit
#
# rm -rf repo &&
# test_create_repo repo &&
# test_commit -C repo foo &&
# test_commit -C repo bar &&
#
# FOO=$(git -C repo rev-parse foo) &&
# promise_and_delete "$FOO" &&
#
# git -C repo config core.repositoryformatversion 1 &&
# git -C repo config extensions.partialclone "arbitrary
string" &&
# GIT_TEST_COMMIT_GRAPH=0 git -C repo rev-list
--exclude-promisor-objects --objects bar >out &&
# grep $(git -C repo rev-parse bar) out &&
# ! grep $FOO out
#
Cheers,
Randall
-- Brief whoami:
NonStop developer since approximately 211288444200000000
UNIX developer since approximately 421664400
-- In my real life, I talk too much.
^ permalink raw reply
* Re: [PATCH v2 2/7] test-lib: parse some --options earlier
From: SZEDER Gábor @ 2018-12-30 19:04 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano
In-Reply-To: <20181217214436.GB14251@sigill.intra.peff.net>
On Mon, Dec 17, 2018 at 04:44:36PM -0500, Jeff King wrote:
> On Tue, Dec 11, 2018 at 01:42:45PM +0100, SZEDER Gábor wrote:
> > diff --git a/t/test-lib.sh b/t/test-lib.sh
> > index 9a3f7930a3..efdb6be3c8 100644
> > --- a/t/test-lib.sh
> > +++ b/t/test-lib.sh
> > @@ -264,58 +264,65 @@ test "x$TERM" != "xdumb" && (
> > ) &&
> > color=t
> >
> > -while test "$#" -ne 0
> > +store_arg_to=
> > +prev_opt=
> > +for opt
> > do
> > - case "$1" in
> > + if test -n "$store_arg_to"
> > + then
> > + eval $store_arg_to=\$opt
> > + store_arg_to=
> > + prev_opt=
> > + continue
> > + fi
>
> OK, so this is set for the unstuck options, which then pick up the
> option in the next loop iteration. That's perhaps less gross than my
> "re-build the options with set --" trick.
>
> A simple variable set is enough for "-r". In theory we could make this:
>
> if test -n "$handle_unstuck_arg"
> then
> eval "$handle_unstuck_arg \$1"
> fi
> ...
>
> -r)
> handle_unstuck_arg=handle_opt_r ;;
>
> and handle_opt_r() could do whatever it wants. But I don't really
> foresee us adding a lot of new options
Yeah, I would refrain from making it too general and fancy with a
callback function for now, when there is only a single option that
could use it.
> (in fact, given that this is just
> the internal tests, I am tempted to say that we should just make it
> "-r<arg>" for the sake of simplicity and consistency. But maybe somebody
> would be annoyed. I have never used "-r" ever myself).
I didn't even know what '-r' does...
And I agree that changing it to '-r<arg>' would be the best, but this
patch series is about adding '--stress', so changing how '-r' gets its
mandatory argument (and potentially annoying someone) is beyond the
scope, I would say.
^ permalink raw reply
* [PATCH v3 0/8] test-lib: add the '--stress' option to help reproduce occasional failures in flaky tests
From: SZEDER Gábor @ 2018-12-30 19:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git, SZEDER Gábor
In-Reply-To: <20181209225628.22216-1-szeder.dev@gmail.com>
To recap: this patch series tries to make reproducing rare failures in
flaky tests easier: it adds the '--stress' option to our test library
to run the test script repeatedly in multiple parallel jobs, in the
hope that the increased load creates enough variance in the timing of
the test's commands that such a failure is eventually triggered.
Changes since v2:
- Most importantly, parse all options before handling '--tee',
'--verbose-log' or '--stress'. This constitutes the bulk of the
range diff.
- Add a bit of sanity check for the argument of '--stress=123'.
- Minor commit message updates.
v2 was quickly followed up with a fixup! patch; the range-diff below
includes that fixup! already squashed in.
Previous versions:
v2: https://public-inbox.org/git/20181209225628.22216-1-szeder.dev@gmail.com/T/#u
v1: https://public-inbox.org/git/20181204163457.15717-1-szeder.dev@gmail.com/T/
SZEDER Gábor (8):
test-lib: translate SIGTERM and SIGHUP to an exit
test-lib: parse options in a for loop to keep $@ intact
test-lib: parse command line options earlier
test-lib: consolidate naming of test-results paths
test-lib: set $TRASH_DIRECTORY earlier
test-lib: extract Bash version check for '-x' tracing
test-lib-functions: introduce the 'test_set_port' helper function
test-lib: add the '--stress' option to run a test repeatedly under
load
t/README | 19 ++-
t/lib-git-daemon.sh | 2 +-
t/lib-git-p4.sh | 9 +-
t/lib-git-svn.sh | 2 +-
t/lib-httpd.sh | 2 +-
t/t0410-partial-clone.sh | 1 -
t/t5512-ls-remote.sh | 2 +-
t/test-lib-functions.sh | 39 +++++
t/test-lib.sh | 360 ++++++++++++++++++++++++++-------------
9 files changed, 303 insertions(+), 133 deletions(-)
Range-diff:
1: 3a5c926167 = 1: 48a2b19218 test-lib: translate SIGTERM and SIGHUP to an exit
2: 8eee8d7fba < -: ---------- test-lib: parse some --options earlier
-: ---------- > 2: adc5e8a86e test-lib: parse options in a for loop to keep $@ intact
-: ---------- > 3: 89748074db test-lib: parse command line options earlier
3: dd20ae5e9a ! 4: 5a6d17f54a test-lib: consolidate naming of test-results paths
@@ -16,8 +16,8 @@
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@
- esac
- done
+ verbose=t
+ fi
+TEST_NAME="$(basename "$0" .sh)"
+TEST_RESULTS_DIR="$TEST_OUTPUT_DIRECTORY/test-results"
@@ -27,8 +27,8 @@
# additionally to the file test-results/$BASENAME.out, too.
if test "$GIT_TEST_TEE_STARTED" = "done"
@@
- elif test -n "$tee" || test -n "$verbose_log" ||
- test -n "$valgrind" || test -n "$valgrind_only"
+ : # do not redirect again
+ elif test -n "$tee"
then
- mkdir -p "$TEST_OUTPUT_DIRECTORY/test-results"
- BASE="$TEST_OUTPUT_DIRECTORY/test-results/$(basename "$0" .sh)"
4: f6e56c91c4 ! 5: cd7626b782 test-lib: set $TRASH_DIRECTORY earlier
@@ -15,15 +15,6 @@
diff --git a/t/test-lib.sh b/t/test-lib.sh
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
-@@
- valgrind=${opt#--*=} ;;
- --valgrind-only=*)
- valgrind_only=${opt#--*=} ;;
-+ --root=*)
-+ root=${opt#--*=} ;;
- *)
- # Other options will be handled later.
- esac
@@
TEST_NAME="$(basename "$0" .sh)"
TEST_RESULTS_DIR="$TEST_OUTPUT_DIRECTORY/test-results"
@@ -37,26 +28,6 @@
# if --tee was passed, write the output not only to the terminal, but
# additionally to the file test-results/$BASENAME.out, too.
-@@
- with_dashes=t; shift ;;
- --no-color)
- color=; shift ;;
-- --root=*)
-- root=${1#--*=}
-- shift ;;
- --chain-lint)
- GIT_TEST_CHAIN_LINT=1
- shift ;;
-@@
- -V|--verbose-log|\
- --va|--val|--valg|--valgr|--valgri|--valgrin|--valgrind|\
- --valgrind=*|\
-- --valgrind-only=*)
-+ --valgrind-only=*|\
-+ --root=*)
- shift ;; # These options were handled already.
- *)
- echo "error: unknown test option '$1'" >&2; exit 1 ;;
@@
fi
5: 99ecd2902e ! 6: 90296ac776 test-lib: extract Bash version check for '-x' tracing
@@ -2,23 +2,30 @@
test-lib: extract Bash version check for '-x' tracing
- Some of our test scripts can't be reliably run with '-x' tracing
- enabled unless executed with a Bash version supporting BASH_XTRACEFD
- (since v4.1), and we have a lengthy condition to disable tracing if
- such a test script is not executed with a suitable Bash version.
+ One of our test scripts, 't1510-repo-setup.sh' [1], still can't be
+ reliably run with '-x' tracing enabled, unless it's executed with a
+ Bash version supporting BASH_XTRACEFD (since v4.1). We have a lengthy
+ condition to check the version of the shell running the test script,
+ and disable tracing if it's not executed with a suitable Bash version
+ [2].
Move this check out from the option parsing loop, so other options can
imply '-x' by setting 'trace=t', without missing this Bash version
check.
+ [1] 5827506928 (t1510-repo-setup: mark as untraceable with '-x',
+ 2018-02-24)
+ [2] 5fc98e79fc (t: add means to disable '-x' tracing for individual
+ test scripts, 2018-02-24)
+
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
diff --git a/t/test-lib.sh b/t/test-lib.sh
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@
- GIT_TEST_CHAIN_LINT=0
- shift ;;
+ --no-chain-lint)
+ GIT_TEST_CHAIN_LINT=0 ;;
-x)
- # Some test scripts can't be reliably traced with '-x',
- # unless the test is run with a Bash version supporting
@@ -38,10 +45,11 @@
- else
- echo >&2 "warning: ignoring -x; '$0' is untraceable without BASH_XTRACEFD"
- fi
-+ trace=t
- shift ;;
- --tee|\
- -V|--verbose-log|\
+- ;;
++ trace=t ;;
+ -V|--verbose-log)
+ verbose_log=t
+ tee=t
@@
test -z "$verbose_log" && verbose=t
fi
6: dcf7d2a397 ! 7: a4a3e7cefa test-lib-functions: introduce the 'test_set_port' helper function
@@ -178,7 +178,7 @@
+
+ eval $var=$port
+ ;;
-+ *[^0-9]*)
++ *[^0-9]*|0*)
+ error >&7 "invalid port number: $port"
+ ;;
+ *)
7: f27f2fc1d0 ! 8: b0dcff0d3f test-lib: add the '--stress' option to run a test repeatedly under load
@@ -4,12 +4,12 @@
Unfortunately, we have a few flaky tests, whose failures tend to be
hard to reproduce. We've found that the best we can do to reproduce
- such a failure is to run the test repeatedly while the machine is
- under load, and wait in the hope that the load creates enough variance
- in the timing of the test's commands that a failure is evenually
- triggered. I have a command to do that, and I noticed that two other
- contributors have rolled their own scripts to do the same, all
- choosing slightly different approaches.
+ such a failure is to run the test script repeatedly while the machine
+ is under load, and wait in the hope that the load creates enough
+ variance in the timing of the test's commands that a failure is
+ evenually triggered. I have a command to do that, and I noticed that
+ two other contributors have rolled their own scripts to do the same,
+ all choosing slightly different approaches.
To help reproduce failures in flaky tests, introduce the '--stress'
option to run a test script repeatedly in multiple parallel jobs until
@@ -37,19 +37,21 @@
user or to the test number, so even tests involving daemons
listening on a TCP socket can be stressed.
- - Redirect each parallel test run's output to
+ - Redirect each parallel test run's verbose output to
't/test-results/$TEST_NAME.stress-<nr>.out', because dumping the
output of several parallel running tests to the terminal would
create a big ugly mess.
For convenience, print the output of the failed test job at the end,
and rename its trash directory to end with the '.stress-failed'
- suffix, so it's easy to find in a predictable path. If, in an
- unlikely case, more than one jobs were to fail nearly at the same
- time, then print the output of all failed jobs, and rename the trash
- directory of only the last one (i.e. with the highest job number), as
- it is the trash directory of the test whose output will be at the
- bottom of the user's terminal.
+ suffix, so it's easy to find in a predictable path (OTOH, all absolute
+ paths recorded in the trash directory become invalid; we'll see
+ whether this causes any issues in practice). If, in an unlikely case,
+ more than one jobs were to fail nearly at the same time, then print
+ the output of all failed jobs, and rename the trash directory of only
+ the last one (i.e. with the highest job number), as it is the trash
+ directory of the test whose output will be at the bottom of the user's
+ terminal.
Based on Jeff King's 'stress' script.
@@ -102,7 +104,7 @@
-
- eval $var=$port
;;
- *[^0-9]*)
+ *[^0-9]*|0*)
error >&7 "invalid port number: $port"
@@
# The user has specified the port.
@@ -119,17 +121,42 @@
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@
- valgrind_only=${opt#--*=} ;;
- --root=*)
- root=${opt#--*=} ;;
+ verbose_log=t
+ tee=t
+ ;;
+ --stress)
+ stress=t ;;
+ --stress=*)
-+ stress=${opt#--*=} ;;
++ stress=${opt#--*=}
++ case "$stress" in
++ *[^0-9]*|0*|"")
++ echo "error: --stress=<N> requires the number of jobs to run" >&2
++ exit 1
++ ;;
++ *) # Good.
++ ;;
++ esac
++ ;;
*)
- # Other options will be handled later.
+ echo "error: unknown test option '$opt'" >&2; exit 1 ;;
esac
- done
+@@
+ test -z "$verbose_log" && verbose=t
+ fi
+
++if test -n "$stress"
++then
++ verbose=t
++ trace=t
++ immediate=t
++fi
++
+ if test -n "$trace" && test -n "$test_untraceable"
+ then
+ # '-x' tracing requested, but this test script can't be reliably
+@@
+ verbose=t
+ fi
+TEST_STRESS_JOB_SFX="${GIT_TEST_STRESS_JOB_NR:+.stress-$GIT_TEST_STRESS_JOB_NR}"
TEST_NAME="$(basename "$0" .sh)"
@@ -231,27 +258,3 @@
# if --tee was passed, write the output not only to the terminal, but
# additionally to the file test-results/$BASENAME.out, too.
if test "$GIT_TEST_TEE_STARTED" = "done"
-@@
- --va|--val|--valg|--valgr|--valgri|--valgrin|--valgrind|\
- --valgrind=*|\
- --valgrind-only=*|\
-- --root=*)
-+ --root=*|\
-+ --stress|--stress=*)
- shift ;; # These options were handled already.
- *)
- echo "error: unknown test option '$1'" >&2; exit 1 ;;
-@@
- verbose=t
- fi
-
-+if test -n "$stress"
-+then
-+ verbose=t
-+ trace=t
-+ immediate=t
-+fi
-+
- if test -n "$color"
- then
- # Save the color control sequences now rather than run tput
--
2.20.1.151.gec613c4b75
^ permalink raw reply
* [PATCH v3 1/8] test-lib: translate SIGTERM and SIGHUP to an exit
From: SZEDER Gábor @ 2018-12-30 19:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git, SZEDER Gábor
In-Reply-To: <20181230191629.3232-1-szeder.dev@gmail.com>
Right now if a test script receives SIGTERM or SIGHUP (e.g., because a
test was hanging and the user 'kill'-ed it or simply closed the
terminal window the test was running in), the shell exits immediately.
This can be annoying if the test script did any global setup, like
starting apache or git-daemon, as it will not have an opportunity to
clean up after itself. A subsequent run of the test won't be able to
start its own daemon, and will either fail or skip the tests.
Instead, let's trap SIGTERM and SIGHUP as well to make sure we do a
clean shutdown, and just chain it to a normal exit (which will trigger
any cleanup).
This patch follows suit of da706545f7 (t: translate SIGINT to an exit,
2015-03-13), and even stole its commit message as well.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
t/test-lib.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0f1faa24b2..9a3f7930a3 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -476,7 +476,7 @@ die () {
GIT_EXIT_OK=
trap 'die' EXIT
-trap 'exit $?' INT
+trap 'exit $?' INT TERM HUP
# The user-facing functions are loaded from a separate file so that
# test_perf subshells can have them too
--
2.20.1.151.gec613c4b75
^ permalink raw reply related
* [PATCH v3 2/8] test-lib: parse options in a for loop to keep $@ intact
From: SZEDER Gábor @ 2018-12-30 19:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git, SZEDER Gábor
In-Reply-To: <20181230191629.3232-1-szeder.dev@gmail.com>
'test-lib.sh' looks for the presence of certain options like '--tee'
and '--verbose-log', so it can execute the test script again to save
its standard output and error, and to do so it needs the original
command line options the test was invoked with.
The next patch is about to move the option parsing loop earlier in
'test-lib.sh', but it is implemented using 'shift' in a while loop,
effecively destroying "$@" by the end of the option parsing. Not
good.
As a preparatory step, turn that option parsing loop into a 'for opt
in "$@"' loop to preserve "$@" intact while iterating over the
options, and taking extra care to handle the '-r' option's required
argument (or the lack thereof).
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
t/test-lib.sh | 77 ++++++++++++++++++++++++++++-----------------------
1 file changed, 42 insertions(+), 35 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 9a3f7930a3..ed7267962f 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -264,58 +264,59 @@ test "x$TERM" != "xdumb" && (
) &&
color=t
-while test "$#" -ne 0
+store_arg_to=
+prev_opt=
+for opt
do
- case "$1" in
+ if test -n "$store_arg_to"
+ then
+ eval $store_arg_to=\$opt
+ store_arg_to=
+ prev_opt=
+ continue
+ fi
+
+ case "$opt" in
-d|--d|--de|--deb|--debu|--debug)
- debug=t; shift ;;
+ debug=t ;;
-i|--i|--im|--imm|--imme|--immed|--immedi|--immedia|--immediat|--immediate)
- immediate=t; shift ;;
+ immediate=t ;;
-l|--l|--lo|--lon|--long|--long-|--long-t|--long-te|--long-tes|--long-test|--long-tests)
- GIT_TEST_LONG=t; export GIT_TEST_LONG; shift ;;
+ GIT_TEST_LONG=t; export GIT_TEST_LONG ;;
-r)
- shift; test "$#" -ne 0 || {
- echo 'error: -r requires an argument' >&2;
- exit 1;
- }
- run_list=$1; shift ;;
+ store_arg_to=run_list
+ ;;
--run=*)
- run_list=${1#--*=}; shift ;;
+ run_list=${opt#--*=} ;;
-h|--h|--he|--hel|--help)
- help=t; shift ;;
+ help=t ;;
-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
- verbose=t; shift ;;
+ verbose=t ;;
--verbose-only=*)
- verbose_only=${1#--*=}
- shift ;;
+ verbose_only=${opt#--*=}
+ ;;
-q|--q|--qu|--qui|--quie|--quiet)
# Ignore --quiet under a TAP::Harness. Saying how many tests
# passed without the ok/not ok details is always an error.
- test -z "$HARNESS_ACTIVE" && quiet=t; shift ;;
+ test -z "$HARNESS_ACTIVE" && quiet=t ;;
--with-dashes)
- with_dashes=t; shift ;;
+ with_dashes=t ;;
--no-color)
- color=; shift ;;
+ color= ;;
--va|--val|--valg|--valgr|--valgri|--valgrin|--valgrind)
- valgrind=memcheck
- shift ;;
+ valgrind=memcheck ;;
--valgrind=*)
- valgrind=${1#--*=}
- shift ;;
+ valgrind=${opt#--*=} ;;
--valgrind-only=*)
- valgrind_only=${1#--*=}
- shift ;;
+ valgrind_only=${opt#--*=} ;;
--tee)
- shift ;; # was handled already
+ ;; # was handled already
--root=*)
- root=${1#--*=}
- shift ;;
+ root=${opt#--*=} ;;
--chain-lint)
- GIT_TEST_CHAIN_LINT=1
- shift ;;
+ GIT_TEST_CHAIN_LINT=1 ;;
--no-chain-lint)
- GIT_TEST_CHAIN_LINT=0
- shift ;;
+ GIT_TEST_CHAIN_LINT=0 ;;
-x)
# Some test scripts can't be reliably traced with '-x',
# unless the test is run with a Bash version supporting
@@ -335,14 +336,20 @@ do
else
echo >&2 "warning: ignoring -x; '$0' is untraceable without BASH_XTRACEFD"
fi
- shift ;;
+ ;;
-V|--verbose-log)
- verbose_log=t
- shift ;;
+ verbose_log=t ;;
*)
- echo "error: unknown test option '$1'" >&2; exit 1 ;;
+ echo "error: unknown test option '$opt'" >&2; exit 1 ;;
esac
+
+ prev_opt=$opt
done
+if test -n "$store_arg_to"
+then
+ echo "error: $prev_opt requires an argument" >&2
+ exit 1
+fi
if test -n "$valgrind_only"
then
--
2.20.1.151.gec613c4b75
^ permalink raw reply related
* [PATCH v3 3/8] test-lib: parse command line options earlier
From: SZEDER Gábor @ 2018-12-30 19:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git, SZEDER Gábor
In-Reply-To: <20181230191629.3232-1-szeder.dev@gmail.com>
'test-lib.sh' looks for the presence of certain options like '--tee'
and '--verbose-log', so it can execute the test script again to save
its standard output and error. It looks for '--valgrind' as well, to
set up some Valgrind-specific stuff. These all happen before the
actual option parsing loop, and the conditions looking for these
options look a bit odd, too. They are not completely correct, either,
because in a bogus invocation like './t1234-foo.sh -r --tee' they
recognize '--tee', although it should be handled as the required
argument of the '-r' option. This patch series will add two more
options to look out for early, and, in addition, will have to extract
these options' stuck arguments (i.e. '--opt=arg') as well.
So let's move the option parsing loop and the couple of related
conditions following it earlier in 'test-lib.sh', before the place
where the test script is executed again for '--tee' and its friends.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
t/test-lib.sh | 228 ++++++++++++++++++++++++++------------------------
1 file changed, 119 insertions(+), 109 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index ed7267962f..fcc04afdff 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -71,13 +71,125 @@ then
exit 1
fi
+# Parse options while taking care to leave $@ intact, so we will still
+# have all the original command line options when executing the test
+# script again for '--tee' and '--verbose-log' below.
+store_arg_to=
+prev_opt=
+for opt
+do
+ if test -n "$store_arg_to"
+ then
+ eval $store_arg_to=\$opt
+ store_arg_to=
+ prev_opt=
+ continue
+ fi
+
+ case "$opt" in
+ -d|--d|--de|--deb|--debu|--debug)
+ debug=t ;;
+ -i|--i|--im|--imm|--imme|--immed|--immedi|--immedia|--immediat|--immediate)
+ immediate=t ;;
+ -l|--l|--lo|--lon|--long|--long-|--long-t|--long-te|--long-tes|--long-test|--long-tests)
+ GIT_TEST_LONG=t; export GIT_TEST_LONG ;;
+ -r)
+ store_arg_to=run_list
+ ;;
+ --run=*)
+ run_list=${opt#--*=} ;;
+ -h|--h|--he|--hel|--help)
+ help=t ;;
+ -v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
+ verbose=t ;;
+ --verbose-only=*)
+ verbose_only=${opt#--*=}
+ ;;
+ -q|--q|--qu|--qui|--quie|--quiet)
+ # Ignore --quiet under a TAP::Harness. Saying how many tests
+ # passed without the ok/not ok details is always an error.
+ test -z "$HARNESS_ACTIVE" && quiet=t ;;
+ --with-dashes)
+ with_dashes=t ;;
+ --no-color)
+ color= ;;
+ --va|--val|--valg|--valgr|--valgri|--valgrin|--valgrind)
+ valgrind=memcheck
+ tee=t
+ ;;
+ --valgrind=*)
+ valgrind=${opt#--*=}
+ tee=t
+ ;;
+ --valgrind-only=*)
+ valgrind_only=${opt#--*=}
+ tee=t
+ ;;
+ --tee)
+ tee=t ;;
+ --root=*)
+ root=${opt#--*=} ;;
+ --chain-lint)
+ GIT_TEST_CHAIN_LINT=1 ;;
+ --no-chain-lint)
+ GIT_TEST_CHAIN_LINT=0 ;;
+ -x)
+ # Some test scripts can't be reliably traced with '-x',
+ # unless the test is run with a Bash version supporting
+ # BASH_XTRACEFD (introduced in Bash v4.1). Check whether
+ # this test is marked as such, and ignore '-x' if it
+ # isn't executed with a suitable Bash version.
+ if test -z "$test_untraceable" || {
+ test -n "$BASH_VERSION" && {
+ test ${BASH_VERSINFO[0]} -gt 4 || {
+ test ${BASH_VERSINFO[0]} -eq 4 &&
+ test ${BASH_VERSINFO[1]} -ge 1
+ }
+ }
+ }
+ then
+ trace=t
+ else
+ echo >&2 "warning: ignoring -x; '$0' is untraceable without BASH_XTRACEFD"
+ fi
+ ;;
+ -V|--verbose-log)
+ verbose_log=t
+ tee=t
+ ;;
+ *)
+ echo "error: unknown test option '$opt'" >&2; exit 1 ;;
+ esac
+
+ prev_opt=$opt
+done
+if test -n "$store_arg_to"
+then
+ echo "error: $prev_opt requires an argument" >&2
+ exit 1
+fi
+
+if test -n "$valgrind_only"
+then
+ test -z "$valgrind" && valgrind=memcheck
+ test -z "$verbose" && verbose_only="$valgrind_only"
+elif test -n "$valgrind"
+then
+ test -z "$verbose_log" && verbose=t
+fi
+
+if test -n "$trace" && test -z "$verbose_log"
+then
+ verbose=t
+fi
+
# if --tee was passed, write the output not only to the terminal, but
# additionally to the file test-results/$BASENAME.out, too.
-case "$GIT_TEST_TEE_STARTED, $* " in
-done,*)
- # do not redirect again
- ;;
-*' --tee '*|*' --va'*|*' -V '*|*' --verbose-log '*)
+if test "$GIT_TEST_TEE_STARTED" = "done"
+then
+ : # do not redirect again
+elif test -n "$tee"
+then
mkdir -p "$TEST_OUTPUT_DIRECTORY/test-results"
BASE="$TEST_OUTPUT_DIRECTORY/test-results/$(basename "$0" .sh)"
@@ -94,8 +206,7 @@ done,*)
echo $? >"$BASE.exit") | tee -a "$GIT_TEST_TEE_OUTPUT_FILE"
test "$(cat "$BASE.exit")" = 0
exit
- ;;
-esac
+fi
# For repeatability, reset the environment to known value.
# TERM is sanitized below, after saving color control sequences.
@@ -193,7 +304,7 @@ fi
# Add libc MALLOC and MALLOC_PERTURB test
# only if we are not executing the test with valgrind
-if expr " $GIT_TEST_OPTS " : ".* --valgrind " >/dev/null ||
+if test -n "$valgrind" ||
test -n "$TEST_NO_MALLOC_CHECK"
then
setup_malloc_check () {
@@ -264,107 +375,6 @@ test "x$TERM" != "xdumb" && (
) &&
color=t
-store_arg_to=
-prev_opt=
-for opt
-do
- if test -n "$store_arg_to"
- then
- eval $store_arg_to=\$opt
- store_arg_to=
- prev_opt=
- continue
- fi
-
- case "$opt" in
- -d|--d|--de|--deb|--debu|--debug)
- debug=t ;;
- -i|--i|--im|--imm|--imme|--immed|--immedi|--immedia|--immediat|--immediate)
- immediate=t ;;
- -l|--l|--lo|--lon|--long|--long-|--long-t|--long-te|--long-tes|--long-test|--long-tests)
- GIT_TEST_LONG=t; export GIT_TEST_LONG ;;
- -r)
- store_arg_to=run_list
- ;;
- --run=*)
- run_list=${opt#--*=} ;;
- -h|--h|--he|--hel|--help)
- help=t ;;
- -v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
- verbose=t ;;
- --verbose-only=*)
- verbose_only=${opt#--*=}
- ;;
- -q|--q|--qu|--qui|--quie|--quiet)
- # Ignore --quiet under a TAP::Harness. Saying how many tests
- # passed without the ok/not ok details is always an error.
- test -z "$HARNESS_ACTIVE" && quiet=t ;;
- --with-dashes)
- with_dashes=t ;;
- --no-color)
- color= ;;
- --va|--val|--valg|--valgr|--valgri|--valgrin|--valgrind)
- valgrind=memcheck ;;
- --valgrind=*)
- valgrind=${opt#--*=} ;;
- --valgrind-only=*)
- valgrind_only=${opt#--*=} ;;
- --tee)
- ;; # was handled already
- --root=*)
- root=${opt#--*=} ;;
- --chain-lint)
- GIT_TEST_CHAIN_LINT=1 ;;
- --no-chain-lint)
- GIT_TEST_CHAIN_LINT=0 ;;
- -x)
- # Some test scripts can't be reliably traced with '-x',
- # unless the test is run with a Bash version supporting
- # BASH_XTRACEFD (introduced in Bash v4.1). Check whether
- # this test is marked as such, and ignore '-x' if it
- # isn't executed with a suitable Bash version.
- if test -z "$test_untraceable" || {
- test -n "$BASH_VERSION" && {
- test ${BASH_VERSINFO[0]} -gt 4 || {
- test ${BASH_VERSINFO[0]} -eq 4 &&
- test ${BASH_VERSINFO[1]} -ge 1
- }
- }
- }
- then
- trace=t
- else
- echo >&2 "warning: ignoring -x; '$0' is untraceable without BASH_XTRACEFD"
- fi
- ;;
- -V|--verbose-log)
- verbose_log=t ;;
- *)
- echo "error: unknown test option '$opt'" >&2; exit 1 ;;
- esac
-
- prev_opt=$opt
-done
-if test -n "$store_arg_to"
-then
- echo "error: $prev_opt requires an argument" >&2
- exit 1
-fi
-
-if test -n "$valgrind_only"
-then
- test -z "$valgrind" && valgrind=memcheck
- test -z "$verbose" && verbose_only="$valgrind_only"
-elif test -n "$valgrind"
-then
- test -z "$verbose_log" && verbose=t
-fi
-
-if test -n "$trace" && test -z "$verbose_log"
-then
- verbose=t
-fi
-
if test -n "$color"
then
# Save the color control sequences now rather than run tput
--
2.20.1.151.gec613c4b75
^ permalink raw reply related
* [PATCH v3 6/8] test-lib: extract Bash version check for '-x' tracing
From: SZEDER Gábor @ 2018-12-30 19:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git, SZEDER Gábor
In-Reply-To: <20181230191629.3232-1-szeder.dev@gmail.com>
One of our test scripts, 't1510-repo-setup.sh' [1], still can't be
reliably run with '-x' tracing enabled, unless it's executed with a
Bash version supporting BASH_XTRACEFD (since v4.1). We have a lengthy
condition to check the version of the shell running the test script,
and disable tracing if it's not executed with a suitable Bash version
[2].
Move this check out from the option parsing loop, so other options can
imply '-x' by setting 'trace=t', without missing this Bash version
check.
[1] 5827506928 (t1510-repo-setup: mark as untraceable with '-x',
2018-02-24)
[2] 5fc98e79fc (t: add means to disable '-x' tracing for individual
test scripts, 2018-02-24)
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
t/test-lib.sh | 38 +++++++++++++++++++-------------------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 2b88ba2de1..7d77a26d44 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -134,25 +134,7 @@ do
--no-chain-lint)
GIT_TEST_CHAIN_LINT=0 ;;
-x)
- # Some test scripts can't be reliably traced with '-x',
- # unless the test is run with a Bash version supporting
- # BASH_XTRACEFD (introduced in Bash v4.1). Check whether
- # this test is marked as such, and ignore '-x' if it
- # isn't executed with a suitable Bash version.
- if test -z "$test_untraceable" || {
- test -n "$BASH_VERSION" && {
- test ${BASH_VERSINFO[0]} -gt 4 || {
- test ${BASH_VERSINFO[0]} -eq 4 &&
- test ${BASH_VERSINFO[1]} -ge 1
- }
- }
- }
- then
- trace=t
- else
- echo >&2 "warning: ignoring -x; '$0' is untraceable without BASH_XTRACEFD"
- fi
- ;;
+ trace=t ;;
-V|--verbose-log)
verbose_log=t
tee=t
@@ -178,6 +160,24 @@ then
test -z "$verbose_log" && verbose=t
fi
+if test -n "$trace" && test -n "$test_untraceable"
+then
+ # '-x' tracing requested, but this test script can't be reliably
+ # traced, unless it is run with a Bash version supporting
+ # BASH_XTRACEFD (introduced in Bash v4.1).
+ if test -n "$BASH_VERSION" && {
+ test ${BASH_VERSINFO[0]} -gt 4 || {
+ test ${BASH_VERSINFO[0]} -eq 4 &&
+ test ${BASH_VERSINFO[1]} -ge 1
+ }
+ }
+ then
+ : Executed by a Bash version supporting BASH_XTRACEFD. Good.
+ else
+ echo >&2 "warning: ignoring -x; '$0' is untraceable without BASH_XTRACEFD"
+ trace=
+ fi
+fi
if test -n "$trace" && test -z "$verbose_log"
then
verbose=t
--
2.20.1.151.gec613c4b75
^ permalink raw reply related
* [PATCH v3 7/8] test-lib-functions: introduce the 'test_set_port' helper function
From: SZEDER Gábor @ 2018-12-30 19:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git, SZEDER Gábor
In-Reply-To: <20181230191629.3232-1-szeder.dev@gmail.com>
Several test scripts run daemons like 'git-daemon' or Apache, and
communicate with them through TCP sockets. To have unique ports where
these daemons are accessible, the ports are usually the number of the
corresponding test scripts, unless the user overrides them via
environment variables, and thus all those tests and test libs contain
more or less the same bit of one-liner boilerplate code to find out
the port. The last patch in this series will make this a bit more
complicated.
Factor out finding the port for a daemon into the common helper
function 'test_set_port' to avoid repeating ourselves.
Take special care of test scripts with "low" numbers:
- Test numbers below 1024 would result in a port that's only usable
as root, so set their port to '10000 + test-nr' to make sure it
doesn't interfere with other tests in the test suite. This makes
the hardcoded port number in 't0410-partial-clone.sh' unnecessary,
remove it.
- The shell's arithmetic evaluation interprets numbers with leading
zeros as octal values, which means that test number below 1000 and
containing the digits 8 or 9 will trigger an error. Remove all
leading zeros from the test numbers to prevent this.
Note that the 'git p4' tests are unlike the other tests involving
daemons in that:
- 'lib-git-p4.sh' doesn't use the test's number for unique port as
is, but does a bit of additional arithmetic on top [1].
- The port is not overridable via an environment variable.
With this patch even 'git p4' tests will use the test's number as
default port, and it will be overridable via the P4DPORT environment
variable.
[1] Commit fc00233071 (git-p4 tests: refactor and cleanup, 2011-08-22)
introduced that "unusual" unique port computation without
explaining why it was necessary (as opposed to simply using the
test number as is). It seems to be just unnecessary complication,
and in any case that commit came way before the "test nr as unique
port" got "standardized" for other daemons in commits c44132fcf3
(tests: auto-set git-daemon port, 2014-02-10), 3bb486e439 (tests:
auto-set LIB_HTTPD_PORT from test name, 2014-02-10), and
bf9d7df950 (t/lib-git-svn.sh: improve svnserve tests with parallel
make test, 2017-12-01).
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
t/lib-git-daemon.sh | 2 +-
t/lib-git-p4.sh | 9 +--------
t/lib-git-svn.sh | 2 +-
t/lib-httpd.sh | 2 +-
t/t0410-partial-clone.sh | 1 -
t/t5512-ls-remote.sh | 2 +-
t/test-lib-functions.sh | 36 ++++++++++++++++++++++++++++++++++++
7 files changed, 41 insertions(+), 13 deletions(-)
diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh
index f98de95c15..41eb1e3ae8 100644
--- a/t/lib-git-daemon.sh
+++ b/t/lib-git-daemon.sh
@@ -28,7 +28,7 @@ then
test_skip_or_die $GIT_TEST_GIT_DAEMON "file system does not support FIFOs"
fi
-LIB_GIT_DAEMON_PORT=${LIB_GIT_DAEMON_PORT-${this_test#t}}
+test_set_port LIB_GIT_DAEMON_PORT
GIT_DAEMON_PID=
GIT_DAEMON_DOCUMENT_ROOT_PATH="$PWD"/repo
diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index c27599474c..b3be3ba011 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -53,14 +53,7 @@ time_in_seconds () {
(cd / && "$PYTHON_PATH" -c 'import time; print(int(time.time()))')
}
-# Try to pick a unique port: guess a large number, then hope
-# no more than one of each test is running.
-#
-# This does not handle the case where somebody else is running the
-# same tests and has chosen the same ports.
-testid=${this_test#t}
-git_p4_test_start=9800
-P4DPORT=$((10669 + ($testid - $git_p4_test_start)))
+test_set_port P4DPORT
P4PORT=localhost:$P4DPORT
P4CLIENT=client
diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh
index a8130f9119..f3b478c307 100644
--- a/t/lib-git-svn.sh
+++ b/t/lib-git-svn.sh
@@ -13,6 +13,7 @@ fi
GIT_DIR=$PWD/.git
GIT_SVN_DIR=$GIT_DIR/svn/refs/remotes/git-svn
SVN_TREE=$GIT_SVN_DIR/svn-tree
+test_set_port SVNSERVE_PORT
svn >/dev/null 2>&1
if test $? -ne 1
@@ -119,7 +120,6 @@ require_svnserve () {
}
start_svnserve () {
- SVNSERVE_PORT=${SVNSERVE_PORT-${this_test#t}}
svnserve --listen-port $SVNSERVE_PORT \
--root "$rawsvnrepo" \
--listen-once \
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index a8729f8232..e465116ef9 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -82,7 +82,7 @@ case $(uname) in
esac
LIB_HTTPD_PATH=${LIB_HTTPD_PATH-"$DEFAULT_HTTPD_PATH"}
-LIB_HTTPD_PORT=${LIB_HTTPD_PORT-${this_test#t}}
+test_set_port LIB_HTTPD_PORT
TEST_PATH="$TEST_DIRECTORY"/lib-httpd
HTTPD_ROOT_PATH="$PWD"/httpd
diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh
index ba3887f178..0aca8d7588 100755
--- a/t/t0410-partial-clone.sh
+++ b/t/t0410-partial-clone.sh
@@ -480,7 +480,6 @@ test_expect_success 'gc stops traversal when a missing but promised object is re
! grep "$TREE_HASH" out
'
-LIB_HTTPD_PORT=12345 # default port, 410, cannot be used as non-root
. "$TEST_DIRECTORY"/lib-httpd.sh
start_httpd
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index 32e722db2e..cd9e60632d 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -260,7 +260,7 @@ test_lazy_prereq GIT_DAEMON '
# This test spawns a daemon, so run it only if the user would be OK with
# testing with git-daemon.
test_expect_success PIPE,JGIT,GIT_DAEMON 'indicate no refs in standards-compliant empty remote' '
- JGIT_DAEMON_PORT=${JGIT_DAEMON_PORT-${this_test#t}} &&
+ test_set_port JGIT_DAEMON_PORT &&
JGIT_DAEMON_PID= &&
git init --bare empty.git &&
>empty.git/git-daemon-export-ok &&
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 6b3bbf99e4..4459bdda13 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -1263,3 +1263,39 @@ test_oid () {
fi &&
eval "printf '%s' \"\${$var}\""
}
+
+# Choose a port number based on the test script's number and store it in
+# the given variable name, unless that variable already contains a number.
+test_set_port () {
+ local var=$1 port
+
+ if test $# -ne 1 || test -z "$var"
+ then
+ BUG "test_set_port requires a variable name"
+ fi
+
+ eval port=\$$var
+ case "$port" in
+ "")
+ # No port is set in the given env var, use the test
+ # number as port number instead.
+ # Remove not only the leading 't', but all leading zeros
+ # as well, so the arithmetic below won't (mis)interpret
+ # a test number like '0123' as an octal value.
+ port=${this_test#${this_test%%[1-9]*}}
+ if test "${port:-0}" -lt 1024
+ then
+ # root-only port, use a larger one instead.
+ port=$(($port + 10000))
+ fi
+
+ eval $var=$port
+ ;;
+ *[^0-9]*|0*)
+ error >&7 "invalid port number: $port"
+ ;;
+ *)
+ # The user has specified the port.
+ ;;
+ esac
+}
--
2.20.1.151.gec613c4b75
^ permalink raw reply related
* [PATCH v3 5/8] test-lib: set $TRASH_DIRECTORY earlier
From: SZEDER Gábor @ 2018-12-30 19:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git, SZEDER Gábor
In-Reply-To: <20181230191629.3232-1-szeder.dev@gmail.com>
A later patch in this series will need to know the path to the trash
directory early in 'test-lib.sh', but $TRASH_DIRECTORY is set much
later. Furthermore, the path to the trash directory depends on the
'--root=<path>' option, which, too, is parsed too late.
Move parsing '--root=...' to the early option parsing loop, and set
$TRASH_DIRECTORY where the other test-specific path variables are set.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
t/test-lib.sh | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 41457d1dcf..2b88ba2de1 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -186,6 +186,12 @@ fi
TEST_NAME="$(basename "$0" .sh)"
TEST_RESULTS_DIR="$TEST_OUTPUT_DIRECTORY/test-results"
TEST_RESULTS_BASE="$TEST_RESULTS_DIR/$TEST_NAME"
+TRASH_DIRECTORY="trash directory.$TEST_NAME"
+test -n "$root" && TRASH_DIRECTORY="$root/$TRASH_DIRECTORY"
+case "$TRASH_DIRECTORY" in
+/*) ;; # absolute path is good
+ *) TRASH_DIRECTORY="$TEST_OUTPUT_DIRECTORY/$TRASH_DIRECTORY" ;;
+esac
# if --tee was passed, write the output not only to the terminal, but
# additionally to the file test-results/$BASENAME.out, too.
@@ -1046,12 +1052,6 @@ then
fi
# Test repository
-TRASH_DIRECTORY="trash directory.$TEST_NAME"
-test -n "$root" && TRASH_DIRECTORY="$root/$TRASH_DIRECTORY"
-case "$TRASH_DIRECTORY" in
-/*) ;; # absolute path is good
- *) TRASH_DIRECTORY="$TEST_OUTPUT_DIRECTORY/$TRASH_DIRECTORY" ;;
-esac
rm -fr "$TRASH_DIRECTORY" || {
GIT_EXIT_OK=t
echo >&5 "FATAL: Cannot prepare test area"
--
2.20.1.151.gec613c4b75
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox