* [PATCH v3 1/5] http: support file handles for HTTP_KEEP_ERROR
From: Masaya Suzuki @ 2019-01-08 2:47 UTC (permalink / raw)
To: masayasuzuki; +Cc: git, jrnieder, peff, sunshine
In-Reply-To: <20190108024741.62176-1-masayasuzuki@google.com>
HTTP_KEEP_ERROR makes it easy to debug HTTP transport errors. In order
to make HTTP_KEEP_ERROR enabled for all requests, file handles need to
be supported.
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
---
http.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/http.c b/http.c
index 0b6807cef9..06450da96e 100644
--- a/http.c
+++ b/http.c
@@ -1991,16 +1991,19 @@ static int http_request_reauth(const char *url,
/*
* 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.
+ * making our next request.
*/
if (options && options->keep_error) {
switch (target) {
case HTTP_REQUEST_STRBUF:
strbuf_reset(result);
break;
+ case HTTP_REQUEST_FILE:
+ fflush(result);
+ ftruncate(fileno(result), 0);
+ break;
default:
- BUG("HTTP_KEEP_ERROR is only supported with strbufs");
+ BUG("Unknown http_request target");
}
}
--
2.20.1.97.g81188d93c3-goog
^ permalink raw reply related
* [PATCH v3 0/5] Show HTTP headers of failed requests with GIT_CURL_VERBOSE
From: Masaya Suzuki @ 2019-01-08 2:47 UTC (permalink / raw)
To: masayasuzuki; +Cc: git, jrnieder, peff, sunshine
In-Reply-To: <20181229194447.157763-1-masayasuzuki@google.com>
Diff from v2[1]:
* Remove http_resonse_dest
This was introduced to have a filename for freopen. Jeff King proposed [2]
using fflush and ftruncate and this makes this struct not needed.
* Unset CURLOPT_FAILONERROR only when it's necessary.
Previously, CURLOPT_FAILONERROR was unset for everything. This patch series
does so only when it's necessary. This is from the observation in [3] that
pointed out there are other possible code paths that hit http.c.
* Split the patches for easier review
[1]: https://public-inbox.org/git/20181229194447.157763-1-masayasuzuki@google.com/
[2]: https://public-inbox.org/git/20190104101149.GA26185@sigill.intra.peff.net/
[3]: https://public-inbox.org/git/20190104104907.GC26185@sigill.intra.peff.net/
Masaya Suzuki (5):
http: support file handles for HTTP_KEEP_ERROR
http: enable keep_error for HTTP requests
remote-curl: define struct for CURLOPT_WRITEFUNCTION
remote-curl: unset CURLOPT_FAILONERROR
test: test GIT_CURL_VERBOSE=1 shows an error
http.c | 27 +++++++++++++--------------
http.h | 1 -
remote-curl.c | 29 ++++++++++++++++++++++++-----
t/lib-httpd/apache.conf | 1 +
t/t5581-http-curl-verbose.sh | 28 ++++++++++++++++++++++++++++
5 files changed, 66 insertions(+), 20 deletions(-)
create mode 100755 t/t5581-http-curl-verbose.sh
--
2.20.1.97.g81188d93c3-goog
^ permalink raw reply
* Re: [PATCH v2 2/2] tree:<depth>: skip some trees even when collecting omits
From: Jonathan Tan @ 2019-01-08 2:00 UTC (permalink / raw)
To: matvore
Cc: git, sbeller, git, jeffhost, peff, stefanbeller, jonathantanmy,
pclouds
In-Reply-To: <20181210234030.176178-3-matvore@google.com>
> -static void filter_trees_update_omits(
> +static int filter_trees_update_omits(
> struct object *obj,
> struct filter_trees_depth_data *filter_data,
> int include_it)
> {
> if (!filter_data->omits)
> - return;
> + return 1;
>
> if (include_it)
> - oidset_remove(filter_data->omits, &obj->oid);
> + return oidset_remove(filter_data->omits, &obj->oid);
> else
> - oidset_insert(filter_data->omits, &obj->oid);
> + return oidset_insert(filter_data->omits, &obj->oid);
> }
I think this function is getting too magical - if filter_data->omits is
not set, we pretend that we have omitted the tree, because we want the
same behavior when not needing omits and when the tree is omitted. Could
this be done another way?
^ permalink raw reply
* Re: [PATCH v2 1/2] list-objects-filter: teach tree:# how to handle >0
From: Jonathan Tan @ 2019-01-08 1:56 UTC (permalink / raw)
To: matvore
Cc: git, sbeller, git, jeffhost, peff, stefanbeller, jonathantanmy,
pclouds
In-Reply-To: <20181210234030.176178-2-matvore@google.com>
Thanks - as stated in your commit message, this adds quite a useful
piece of functionality.
> - case LOFS_BEGIN_TREE:
> - case LOFS_BLOB:
> - if (filter_data->omits) {
> - oidset_insert(filter_data->omits, &obj->oid);
> - /* _MARK_SEEN but not _DO_SHOW (hard omit) */
> - return LOFR_MARK_SEEN;
> - } else {
> - /*
> - * Not collecting omits so no need to to traverse tree.
> - */
> - return LOFR_SKIP_TREE | LOFR_MARK_SEEN;
> - }
> -
> case LOFS_END_TREE:
> assert(obj->type == OBJ_TREE);
> + filter_data->current_depth--;
> return LOFR_ZERO;
>
> + case LOFS_BLOB:
> + filter_trees_update_omits(obj, filter_data, include_it);
> + return include_it ? LOFR_MARK_SEEN | LOFR_DO_SHOW : LOFR_ZERO;
Any reason for moving "case LOFS_BLOB" (and "case LOFS_BEGIN_TREE"
below) after LOFS_END_TREE?
This is drastically different from the previous case, but this makes
sense - previously, all blobs accessed through traversal were not shown,
but now they are sometimes shown. Here, filter_trees_update_omits() is
only ever used to remove a blob from the omits set, since once this blob
is encountered with include_it == true, it is marked as LOFR_MARK_SEEN
and will not be traversed again.
> + case LOFS_BEGIN_TREE:
> + seen_info = oidmap_get(
> + &filter_data->seen_at_depth, &obj->oid);
> + if (!seen_info) {
> + seen_info = xcalloc(1, sizeof(struct seen_map_entry));
Use sizeof(*seen_info).
> + seen_info->base.oid = obj->oid;
We have been using oidcpy, but come to think of it, I'm not sure why...
> + seen_info->depth = filter_data->current_depth;
> + oidmap_put(&filter_data->seen_at_depth, seen_info);
> + already_seen = 0;
> + } else
> + already_seen =
> + filter_data->current_depth >= seen_info->depth;
There has been recently some clarification that if one branch of an
if/else construct requires braces, braces should be put on all of them:
1797dc5176 ("CodingGuidelines: clarify multi-line brace style",
2017-01-17). Likewise below.
> + if (already_seen)
> + filter_res = LOFR_SKIP_TREE;
> + else {
> + seen_info->depth = filter_data->current_depth;
> + filter_trees_update_omits(obj, filter_data, include_it);
> +
> + if (include_it)
> + filter_res = LOFR_DO_SHOW;
> + else if (filter_data->omits)
> + filter_res = LOFR_ZERO;
> + else
> + filter_res = LOFR_SKIP_TREE;
Looks straightforward. If we have already seen it at a shallower or
equal depth, we can skip it (since we have already done the appropriate
processing). Otherwise, we need to ensure that its "omit" is correctly
set, and:
- show it if include_it
- don't do anything special if not include_it and we need the omit set
- skip the tree if not include_it and we don't need the omit set
> +static void filter_trees_free(void *filter_data) {
> + struct filter_trees_depth_data* d = filter_data;
> + oidmap_free(&d->seen_at_depth, 1);
> + free(d);
> +}
Check for NULL-ness of filter_data too, to match the usual behavior of
free functions.
> diff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh
> index eb32505a6e..54e7096d40 100755
> --- a/t/t6112-rev-list-filters-objects.sh
> +++ b/t/t6112-rev-list-filters-objects.sh
[snip]
Thanks for the tests that cover quite a wide range of cases. Can you
also demonstrate the case where a blob would normally be omitted
(because it is too deep) but it is directly specified, so it is
included.
> +expect_has_with_different_name () {
> + repo=$1 &&
> + name=$2 &&
> +
> + hash=$(git -C $repo rev-parse HEAD:$name) &&
> + ! grep "^$hash $name$" actual &&
> + grep "^$hash " actual &&
> + ! grep "~$hash" actual
> +}
Should we also check that a "~" entry appears with $name?
^ permalink raw reply
* Re: [PATCH v2 0/2] support for filtering trees and blobs based on depth
From: Matthew DeVore @ 2019-01-08 0:56 UTC (permalink / raw)
To: Junio C Hamano, Matthew DeVore
Cc: git, sbeller, git, jeffhost, peff, stefanbeller, jonathantanmy,
pclouds
In-Reply-To: <xmqq1s6oladv.fsf@gitster-ct.c.googlers.com>
On 2018/12/11 0:45, Junio C Hamano wrote:
> This seems to require at least two topics still not in 'master';
> I've bookmarked the topic by merging sb/more-repo-in-api and
> nd/the-index into 'master' and then queueing these two patches on
> top, to be able to merge it into 'pu' to see if there are bad
> interactions with other topics and also give others easier access to
> the topic in the integrated form.
>
> Thanks.
I'm re-reading the SubmittingPatches document and now I realize that I
should have based this on master. Thank you for merging things so that
my patch works. Let me know if it would be easier if I rebased this
patch on top of master either now or on the next re-roll.
^ permalink raw reply
* Re: git-p4: default behavior for handling moves?
From: Mazo, Andrey @ 2019-01-08 0:56 UTC (permalink / raw)
To: luke@diamand.org
Cc: chenbin.sh@gmail.com, git@vger.kernel.org, gitster@pobox.com,
merlorom@yahoo.fr, vitor.hda@gmail.com, Mazo, Andrey
In-Reply-To: <CAE5ih7987J2WXdCJvs2e3hOn3zucpE6gsr4JJtxO+XE5=K2G_Q@mail.gmail.com>
> git-p4 can map a "git move" operation to a Perforce "move" operation.
> But by default this is disabled. You then end up with a P4 commit
> where the file is deleted, and a fresh file is created with the same
> contents at the new location at revision #1.
>
> Rename detection gets enabled either with the "-M" option, or with
> some config variables, git-p4.detectCopies and git-p4.detectRenames.
>
> I've been tripped up by this, and I actually know about it, and I know
> other people have been as well.
>
> Should we switch the default over so that it's enabled by default? I
> can't think of any reason why you wouldn't want it enabled.
I have it enabled in my ~/.gitconfig,
so enabling it by default makes total sense to me.
Regarding potential problems,
I occasionally get a wrong "source" file detection.
(e.g. there was `cp a b ; git add b`, but some other file "c" is selected as the source instead)
Or there is a "copy/move" detected, when there was actually none.
But I've only seen this with quite small files (like a trivial one line shell script) or binaries,
and mostly because I have git-p4.detectCopiesHarder set as well as a pretty aggressive git-p4.detectCopies threshold.
(normally 30%, but down to 10% at times to make sure a copy/move is really detected)
But anyway, enabling both git-p4.detect{Copies,Renames}
with default 50% similarity index makes sense to me.
It's probably worth adding command-line options to override the new to-be defaults though.
A more conservative approach like only enabling git-p4.detectRenames = 70% is an option too.
>
> I think the rename code was first introduced around 2011 by Vitor.
>
> Another option is to add a warning, but people just ignore warnings!
When such a warning would be shown?
Just before `p4 submit`?
I think, It might be hard to notice for large changesets.
Thank you,
Andrey
^ permalink raw reply
* [PATCH v2 1/1] filter-options: Expand abbreviated numbers
From: Josh Steadmon @ 2019-01-08 0:17 UTC (permalink / raw)
To: git; +Cc: gitster
In-Reply-To: <cover.1546906008.git.steadmon@google.com>
When communicating with a remote server or a subprocess, use expanded
numbers rather than abbreviated numbers in the object filter spec (e.g.
"limit:blob=1k" becomes "limit:blob=1024").
Update the protocol docs to note that clients should always perform this
expansion, to allow for more compatibility between server
implementations.
Signed-off-by: Josh Steadmon <steadmon@google.com>
---
Documentation/technical/protocol-v2.txt | 8 +++++++-
builtin/clone.c | 6 +++++-
builtin/fetch.c | 7 ++++++-
fetch-pack.c | 15 ++++++++++++---
list-objects-filter-options.c | 20 ++++++++++++++++++--
list-objects-filter-options.h | 17 +++++++++++++++--
t/t6112-rev-list-filters-objects.sh | 17 +++++++++++++++++
transport-helper.c | 13 +++++++++----
upload-pack.c | 7 +++++--
9 files changed, 94 insertions(+), 16 deletions(-)
diff --git a/Documentation/technical/protocol-v2.txt b/Documentation/technical/protocol-v2.txt
index 09e4e0273f..292060a9dc 100644
--- a/Documentation/technical/protocol-v2.txt
+++ b/Documentation/technical/protocol-v2.txt
@@ -296,7 +296,13 @@ included in the client's request:
Request that various objects from the packfile be omitted
using one of several filtering techniques. These are intended
for use with partial clone and partial fetch operations. See
- `rev-list` for possible "filter-spec" values.
+ `rev-list` for possible "filter-spec" values. When communicating
+ with other processes, senders SHOULD translate scaled integers
+ (e.g. "1k") into a fully-expanded form (e.g. "1024") to aid
+ interoperability with older receivers that may not understand
+ newly-invented scaling suffixes. However, receivers SHOULD
+ accept the following suffixes: 'k', 'm', and 'g' for 1024,
+ 1048576, and 1073741824, respectively.
If the 'ref-in-want' feature is advertised, the following argument can
be included in the client's request as well as the potential addition of
diff --git a/builtin/clone.c b/builtin/clone.c
index 15b142d646..8e05e8ad6c 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1130,9 +1130,13 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
option_upload_pack);
if (filter_options.choice) {
+ struct strbuf expanded_filter_spec = STRBUF_INIT;
+ expand_list_objects_filter_spec(&filter_options,
+ &expanded_filter_spec);
transport_set_option(transport, TRANS_OPT_LIST_OBJECTS_FILTER,
- filter_options.filter_spec);
+ expanded_filter_spec.buf);
transport_set_option(transport, TRANS_OPT_FROM_PROMISOR, "1");
+ strbuf_release(&expanded_filter_spec);
}
if (transport->smart_options && !deepen && !filter_options.choice)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index e0140327aa..8b8bb64921 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1172,6 +1172,7 @@ static void add_negotiation_tips(struct git_transport_options *smart_options)
static struct transport *prepare_transport(struct remote *remote, int deepen)
{
struct transport *transport;
+
transport = transport_get(remote, NULL);
transport_set_verbosity(transport, verbosity, progress);
transport->family = family;
@@ -1191,9 +1192,13 @@ static struct transport *prepare_transport(struct remote *remote, int deepen)
if (update_shallow)
set_option(transport, TRANS_OPT_UPDATE_SHALLOW, "yes");
if (filter_options.choice) {
+ struct strbuf expanded_filter_spec = STRBUF_INIT;
+ expand_list_objects_filter_spec(&filter_options,
+ &expanded_filter_spec);
set_option(transport, TRANS_OPT_LIST_OBJECTS_FILTER,
- filter_options.filter_spec);
+ expanded_filter_spec.buf);
set_option(transport, TRANS_OPT_FROM_PROMISOR, "1");
+ strbuf_release(&expanded_filter_spec);
}
if (negotiation_tip.nr) {
if (transport->smart_options)
diff --git a/fetch-pack.c b/fetch-pack.c
index 9691046e64..485632fabe 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -329,9 +329,14 @@ static int find_common(struct fetch_negotiator *negotiator,
packet_buf_write(&req_buf, "deepen-not %s", s->string);
}
}
- if (server_supports_filtering && args->filter_options.choice)
+ if (server_supports_filtering && args->filter_options.choice) {
+ struct strbuf expanded_filter_spec = STRBUF_INIT;
+ expand_list_objects_filter_spec(&args->filter_options,
+ &expanded_filter_spec);
packet_buf_write(&req_buf, "filter %s",
- args->filter_options.filter_spec);
+ expanded_filter_spec.buf);
+ strbuf_release(&expanded_filter_spec);
+ }
packet_buf_flush(&req_buf);
state_len = req_buf.len;
@@ -1155,9 +1160,13 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,
/* Add filter */
if (server_supports_feature("fetch", "filter", 0) &&
args->filter_options.choice) {
+ struct strbuf expanded_filter_spec = STRBUF_INIT;
print_verbose(args, _("Server supports filter"));
+ expand_list_objects_filter_spec(&args->filter_options,
+ &expanded_filter_spec);
packet_buf_write(&req_buf, "filter %s",
- args->filter_options.filter_spec);
+ expanded_filter_spec.buf);
+ strbuf_release(&expanded_filter_spec);
} else if (args->filter_options.choice) {
warning("filtering not recognized by server, ignoring");
}
diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c
index 5285e7674d..9efb3e9902 100644
--- a/list-objects-filter-options.c
+++ b/list-objects-filter-options.c
@@ -18,8 +18,9 @@
* See Documentation/rev-list-options.txt for allowed values for <arg>.
*
* Capture the given arg as the "filter_spec". This can be forwarded to
- * subordinate commands when necessary. We also "intern" the arg for
- * the convenience of the current command.
+ * subordinate commands when necessary (although it's better to pass it through
+ * expand_list_objects_filter_spec() first). We also "intern" the arg for the
+ * convenience of the current command.
*/
static int gently_parse_list_objects_filter(
struct list_objects_filter_options *filter_options,
@@ -111,6 +112,21 @@ int opt_parse_list_objects_filter(const struct option *opt,
return parse_list_objects_filter(filter_options, arg);
}
+void expand_list_objects_filter_spec(
+ const struct list_objects_filter_options *filter,
+ struct strbuf *expanded_spec)
+{
+ strbuf_init(expanded_spec, strlen(filter->filter_spec));
+ if (filter->choice == LOFC_BLOB_LIMIT)
+ strbuf_addf(expanded_spec, "blob:limit=%lu",
+ filter->blob_limit_value);
+ else if (filter->choice == LOFC_TREE_DEPTH)
+ strbuf_addf(expanded_spec, "tree:%lu",
+ filter->tree_exclude_depth);
+ else
+ strbuf_addstr(expanded_spec, filter->filter_spec);
+}
+
void list_objects_filter_release(
struct list_objects_filter_options *filter_options)
{
diff --git a/list-objects-filter-options.h b/list-objects-filter-options.h
index 477cd97029..e3adc78ebf 100644
--- a/list-objects-filter-options.h
+++ b/list-objects-filter-options.h
@@ -2,6 +2,7 @@
#define LIST_OBJECTS_FILTER_OPTIONS_H
#include "parse-options.h"
+#include "strbuf.h"
/*
* The list of defined filters for list-objects.
@@ -20,8 +21,9 @@ struct list_objects_filter_options {
/*
* 'filter_spec' is the raw argument value given on the command line
* or protocol request. (The part after the "--keyword=".) For
- * commands that launch filtering sub-processes, this value should be
- * passed to them as received by the current process.
+ * commands that launch filtering sub-processes, or for communication
+ * over the network, don't use this value; use the result of
+ * expand_list_objects_filter_spec() instead.
*/
char *filter_spec;
@@ -62,6 +64,17 @@ int opt_parse_list_objects_filter(const struct option *opt,
N_("object filtering"), 0, \
opt_parse_list_objects_filter }
+/*
+ * Translates abbreviated numbers in the filter's filter_spec into their
+ * fully-expanded forms (e.g., "limit:blob=1k" becomes "limit:blob=1024").
+ *
+ * This form should be used instead of the raw filter_spec field when
+ * communicating with a remote process or subprocess.
+ */
+void expand_list_objects_filter_spec(
+ const struct list_objects_filter_options *filter,
+ struct strbuf *expanded_spec);
+
void list_objects_filter_release(
struct list_objects_filter_options *filter_options);
diff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh
index 18b0b14d5a..f96f551fb5 100755
--- a/t/t6112-rev-list-filters-objects.sh
+++ b/t/t6112-rev-list-filters-objects.sh
@@ -437,4 +437,21 @@ test_expect_success 'rev-list W/ missing=allow-any' '
git -C r1 rev-list --quiet --missing=allow-any --objects HEAD
'
+# Test expansion of filter specs.
+
+test_expect_success 'expand blob limit in protocol' '
+ git -C r2 config --local uploadpack.allowfilter 1 &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -c protocol.version=2 clone \
+ --filter=blob:limit=1k "file://$(pwd)/r2" limit &&
+ ! grep "blob:limit=1k" trace &&
+ grep "blob:limit=1024" trace
+'
+
+test_expect_success 'expand tree depth limit in protocol' '
+ GIT_TRACE_PACKET="$(pwd)/tree_trace" git -c protocol.version=2 clone \
+ --filter=tree:0k "file://$(pwd)/r2" tree &&
+ ! grep "tree:0k" tree_trace &&
+ grep "tree:0" tree_trace
+'
+
test_done
diff --git a/transport-helper.c b/transport-helper.c
index bf225c698f..01404bfac5 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -679,10 +679,15 @@ static int fetch(struct transport *transport,
if (data->transport_options.update_shallow)
set_helper_option(transport, "update-shallow", "true");
- if (data->transport_options.filter_options.choice)
- set_helper_option(
- transport, "filter",
- data->transport_options.filter_options.filter_spec);
+ if (data->transport_options.filter_options.choice) {
+ struct strbuf expanded_filter_spec = STRBUF_INIT;
+ expand_list_objects_filter_spec(
+ &data->transport_options.filter_options,
+ &expanded_filter_spec);
+ set_helper_option(transport, "filter",
+ expanded_filter_spec.buf);
+ strbuf_release(&expanded_filter_spec);
+ }
if (data->transport_options.negotiation_tips)
warning("Ignoring --negotiation-tip because the protocol does not support it.");
diff --git a/upload-pack.c b/upload-pack.c
index 5e81f1ff24..1c6d73e5a2 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -140,14 +140,17 @@ static void create_pack_file(const struct object_array *have_obj,
if (use_include_tag)
argv_array_push(&pack_objects.args, "--include-tag");
if (filter_options.filter_spec) {
+ struct strbuf expanded_filter_spec = STRBUF_INIT;
+ expand_list_objects_filter_spec(&filter_options,
+ &expanded_filter_spec);
if (pack_objects.use_shell) {
struct strbuf buf = STRBUF_INIT;
- sq_quote_buf(&buf, filter_options.filter_spec);
+ sq_quote_buf(&buf, expanded_filter_spec.buf);
argv_array_pushf(&pack_objects.args, "--filter=%s", buf.buf);
strbuf_release(&buf);
} else {
argv_array_pushf(&pack_objects.args, "--filter=%s",
- filter_options.filter_spec);
+ expanded_filter_spec.buf);
}
}
--
2.20.1.97.g81188d93c3-goog
^ permalink raw reply related
* [PATCH v2 0/1] Expand abbreviated filters
From: Josh Steadmon @ 2019-01-08 0:17 UTC (permalink / raw)
To: git; +Cc: gitster
In-Reply-To: <cover.1545261186.git.steadmon@google.com>
NOTE: this patch is based on top of md/list-objects-filter-by-depth
Currently, git clients pass filter specs verbatim over the network and
to subprocesses. We support various scaling suffixes for parameters on
these limits (via git_parse_ulong()), but other implementations may not
support the same suffixes. It would be better to only pass fully-
expanded numbers in this case, and keep the expansion logic completely
on the client side.
This patch updates the protocol-v2 doc to specify that clients SHOULD
expand abbreviations in filter specifications before passing them to
other processes. It adds a new function
"expand_list_objects_filter_spec()" in list-objects-filter-options.c
that implements the expansion logic, and updates users of the
filter_spec field to instead expand the spec first.
Changes since V1:
* Changed "MUST" to "SHOULD" in protocol-v2.txt
* Noted specific suffixes that receivers SHOULD accept
Josh Steadmon (1):
filter-options: Expand abbreviated numbers
Documentation/technical/protocol-v2.txt | 8 +++++++-
builtin/clone.c | 6 +++++-
builtin/fetch.c | 7 ++++++-
fetch-pack.c | 15 ++++++++++++---
list-objects-filter-options.c | 20 ++++++++++++++++++--
list-objects-filter-options.h | 17 +++++++++++++++--
t/t6112-rev-list-filters-objects.sh | 17 +++++++++++++++++
transport-helper.c | 13 +++++++++----
upload-pack.c | 7 +++++--
9 files changed, 94 insertions(+), 16 deletions(-)
Range-diff against v1:
1: d324e78369 ! 1: d35827de35 filter-options: Expand abbreviated numbers
@@ -20,10 +20,13 @@
using one of several filtering techniques. These are intended
for use with partial clone and partial fetch operations. See
- `rev-list` for possible "filter-spec" values.
-+ `rev-list` for possible "filter-spec" values. Clients MUST
-+ translate abbreviated numbers (e.g. "1k") into fully-expanded
-+ numbers (e.g. "1024") on the client side, so that the server
-+ does not need to implement unit parsing.
++ `rev-list` for possible "filter-spec" values. When communicating
++ with other processes, senders SHOULD translate scaled integers
++ (e.g. "1k") into a fully-expanded form (e.g. "1024") to aid
++ interoperability with older receivers that may not understand
++ newly-invented scaling suffixes. However, receivers SHOULD
++ accept the following suffixes: 'k', 'm', and 'g' for 1024,
++ 1048576, and 1073741824, respectively.
If the 'ref-in-want' feature is advertised, the following argument can
be included in the client's request as well as the potential addition of
--
2.20.1.97.g81188d93c3-goog
^ permalink raw reply
* Re: "git p4" fails when perforce login not needed
From: Mazo, Andrey @ 2019-01-07 23:56 UTC (permalink / raw)
To: peterosterlund2@gmail.com
Cc: git@vger.kernel.org, luke@diamand.org, Mazo, Andrey
In-Reply-To: <alpine.LFD.2.21.1901062337420.5908@fractal.localdomain>
On Sun, 6 Jan 2019 at 22:48, Peter Osterlund <peterosterlund2@gmail.com> wrote:
> Hi,
>
> When I use "git p4 sync" to update a git repository from a perforce depot,
> the operation fails with error message:
>
> failure accessing depot: unknown error code info
>
I guess, this got broken in
commit 0ef67acdd78 ("git-p4: better error reporting when p4 fails", 2018-06-08)
> When I run "p4 login -s" from a shell it reports:
>
> 'login' not necessary, no password set for this user.
In my case, I was getting (this is pretty printed `p4 -G` output):
{b'code': b'info', b'level': 0, b'data': b'User <username> was authenticated by password not ticket.'}
I guess, that's because I have P4PASSWD variable exported and/or git-p4.password set.
> The following patch fixes the problem for me:
>
> --- /usr/libexec/git-core/git-p4~ 2018-12-15 14:51:07.000000000 +0100
> +++ /usr/libexec/git-core/git-p4 2019-01-06 23:23:06.934176387 +0100
> @@ -332,6 +332,8 @@
> die_bad_access("p4 error: {0}".format(data))
> else:
> die_bad_access("unknown error")
> + elif code == "info":
> + return
> else:
> die_bad_access("unknown error code {0}".format(code))
This fixes the problem for me too.
>
> Not sure if this helps, but running "p4 -G login -s | hexdump" gives:
>
> 00000000 7b 73 04 00 00 00 63 6f 64 65 73 04 00 00 00 69 |{s....codes....i|
> 00000010 6e 66 6f 73 05 00 00 00 6c 65 76 65 6c 69 00 00 |nfos....leveli..|
> 00000020 00 00 73 04 00 00 00 64 61 74 61 73 35 00 00 00 |..s....datas5...|
> 00000030 27 6c 6f 67 69 6e 27 20 6e 6f 74 20 6e 65 63 65 |'login' not nece|
> 00000040 73 73 61 72 79 2c 20 6e 6f 20 70 61 73 73 77 6f |ssary, no passwo|
> 00000050 72 64 20 73 65 74 20 66 6f 72 20 74 68 69 73 20 |rd set for this |
> 00000060 75 73 65 72 2e 30 |user.0|
> 00000066
>
> Best regards,
^ permalink raw reply
* What's cooking in git.git (Jan 2019, #01; Mon, 7)
From: Junio C Hamano @ 2019-01-07 23:34 UTC (permalink / raw)
To: git
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'. The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.
The 'master' branch now has the first batch of topics (many of which
have been cooking in 'next' during the pre-release freeze).
You can find the changes described here in the integration branches
of the repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--------------------------------------------------
[Graduated to "master"]
* ab/push-dwim-dst (2018-11-14) 7 commits
(merged to 'next' on 2018-12-28 at d9f618de10)
+ push doc: document the DWYM behavior pushing to unqualified <dst>
+ push: test that <src> doesn't DWYM if <dst> is unqualified
+ push: add an advice on unqualified <dst> push
+ push: move unqualified refname error into a function
+ push: improve the error shown on unqualified <dst> push
+ i18n: remote.c: mark error(...) messages for translation
+ remote.c: add braces in anticipation of a follow-up change
Originally merged to 'next' on 2018-11-18
"git push $there $src:$dst" rejects when $dst is not a fully
qualified refname and not clear what the end user meant. The
codepath has been taught to give a clearer error message, and also
guess where the push should go by taking the type of the pushed
object into account (e.g. a tag object would want to go under
refs/tags/).
* en/fast-export-import (2018-11-17) 11 commits
(merged to 'next' on 2018-12-28 at a1b09cf515)
+ fast-export: add a --show-original-ids option to show original names
+ fast-import: remove unmaintained duplicate documentation
+ fast-export: add --reference-excluded-parents option
+ fast-export: ensure we export requested refs
+ fast-export: when using paths, avoid corrupt stream with non-existent mark
+ fast-export: move commit rewriting logic into a function for reuse
+ fast-export: avoid dying when filtering by paths and old tags exist
+ fast-export: use value from correct enum
+ git-fast-export.txt: clarify misleading documentation about rev-list args
+ git-fast-import.txt: fix documentation for --quiet option
+ fast-export: convert sha1 to oid
Originally merged to 'next' on 2018-11-18
Small fixes and features for fast-export and fast-import, mostly on
the fast-export side.
* en/merge-path-collision (2018-12-01) 11 commits
(merged to 'next' on 2018-12-28 at b50d3eee25)
+ t6036: avoid non-portable "cp -a"
+ merge-recursive: combine error handling
+ t6036, t6043: increase code coverage for file collision handling
+ merge-recursive: improve rename/rename(1to2)/add[/add] handling
+ merge-recursive: use handle_file_collision for add/add conflicts
+ merge-recursive: improve handling for rename/rename(2to1) conflicts
+ merge-recursive: fix rename/add conflict handling
+ merge-recursive: new function for better colliding conflict resolutions
+ merge-recursive: increase marker length with depth of recursion
+ t6036, t6042: testcases for rename collision of already conflicting files
+ t6042: add tests for consistency in file collision conflict handling
Originally merged to 'next' on 2018-12-01
Updates for corner cases in merge-recursive.
* fc/http-version (2018-11-09) 1 commit
(merged to 'next' on 2018-12-28 at 56bcbb0fa9)
+ http: add support selecting http version
Originally merged to 'next' on 2018-11-18
The "http.version" configuration variable can be used with recent
enough cURL library to force the version of HTTP used to talk when
fetching and pushing.
* jk/loose-object-cache (2018-11-24) 10 commits
(merged to 'next' on 2018-12-28 at 5a5faf384e)
+ odb_load_loose_cache: fix strbuf leak
+ fetch-pack: drop custom loose object cache
+ sha1-file: use loose object cache for quick existence check
+ object-store: provide helpers for loose_objects_cache
+ sha1-file: use an object_directory for the main object dir
+ handle alternates paths the same as the main object dir
+ sha1_file_name(): overwrite buffer instead of appending
+ rename "alternate_object_database" to "object_directory"
+ submodule--helper: prefer strip_suffix() to ends_with()
+ fsck: do not reuse child_process structs
Originally merged to 'next' on 2018-11-24
Code clean-up with optimization for the codepath that checks
(non-)existence of loose objects.
* mk/http-backend-kill-children-before-exit (2018-11-26) 1 commit
(merged to 'next' on 2018-12-28 at 81188d93c3)
+ http-backend: enable cleaning up forked upload/receive-pack on exit
Originally merged to 'next' on 2018-11-29
The http-backend CGI process did not correctly clean up the child
processes it spawns to run upload-pack etc. when it dies itself,
which has been corrected.
* nd/checkout-dwim-fix (2018-11-14) 1 commit
(merged to 'next' on 2018-12-28 at 3183c9305b)
+ checkout: disambiguate dwim tracking branches and local files
Originally merged to 'next' on 2018-11-18
"git checkout frotz" (without any double-dash) avoids ambiguity by
making sure 'frotz' cannot be interpreted as a revision and as a
path at the same time. This safety has been updated to check also
a unique remote-tracking branch 'frotz' in a remote, when dwimming
to create a local branch 'frotz' out of a remote-tracking branch
'frotz' from a remote.
* nd/i18n (2018-11-12) 16 commits
(merged to 'next' on 2018-12-28 at 8e2de8338e)
+ fsck: mark strings for translation
+ fsck: reduce word legos to help i18n
+ parse-options.c: mark more strings for translation
+ parse-options.c: turn some die() to BUG()
+ parse-options: replace opterror() with optname()
+ repack: mark more strings for translation
+ remote.c: mark messages for translation
+ remote.c: turn some error() or die() to BUG()
+ reflog: mark strings for translation
+ read-cache.c: add missing colon separators
+ read-cache.c: mark more strings for translation
+ read-cache.c: turn die("internal error") to BUG()
+ attr.c: mark more string for translation
+ archive.c: mark more strings for translation
+ alias.c: mark split_cmdline_strerror() strings for translation
+ git.c: mark more strings for translation
Originally merged to 'next' on 2018-11-18
More _("i18n") markings.
* nd/the-index (2018-11-12) 22 commits
(merged to 'next' on 2018-12-28 at 6bbd3befbe)
+ rebase-interactive.c: remove the_repository references
+ rerere.c: remove the_repository references
+ pack-*.c: remove the_repository references
+ pack-check.c: remove the_repository references
+ notes-cache.c: remove the_repository references
+ line-log.c: remove the_repository reference
+ diff-lib.c: remove the_repository references
+ delta-islands.c: remove the_repository references
+ cache-tree.c: remove the_repository references
+ bundle.c: remove the_repository references
+ branch.c: remove the_repository reference
+ bisect.c: remove the_repository reference
+ blame.c: remove implicit dependency the_repository
+ sequencer.c: remove implicit dependency on the_repository
+ sequencer.c: remove implicit dependency on the_index
+ transport.c: remove implicit dependency on the_index
+ notes-merge.c: remove implicit dependency the_repository
+ notes-merge.c: remove implicit dependency on the_index
+ list-objects.c: reduce the_repository references
+ list-objects-filter.c: remove implicit dependency on the_index
+ wt-status.c: remove implicit dependency the_repository
+ wt-status.c: remove implicit dependency on the_index
(this branch is used by md/list-objects-filter-by-depth.)
Originally merged to 'next' on 2018-11-18
More codepaths become aware of working with in-core repository
instance other than the default "the_repository".
* sd/stash-wo-user-name (2018-11-19) 1 commit
(merged to 'next' on 2018-12-28 at 99197ef5a1)
+ stash: tolerate missing user identity
(this branch is used by ps/stash-in-c.)
Originally merged to 'next' on 2018-11-19
A properly configured username/email is required under
user.useConfigOnly in order to create commits; now "git stash"
(even though it creates commit objects to represent stash entries)
command is exempt from the requirement.
* sg/clone-initial-fetch-configuration (2018-11-16) 3 commits
(merged to 'next' on 2018-12-28 at 82e104f221)
+ Documentation/clone: document ignored configuration variables
+ clone: respect additional configured fetch refspecs during initial fetch
+ clone: use a more appropriate variable name for the default refspec
Originally merged to 'next' on 2018-11-18
Refspecs configured with "git -c var=val clone" did not propagate
to the resulting repository, which has been corrected.
--------------------------------------------------
[New Topics]
* ab/proto-v2-fixes (2018-12-14) 5 commits
- tests: mark tests broken under GIT_TEST_PROTOCOL_VERSION=2
- builtin/fetch-pack: support protocol version 2
- tests: mark & fix tests broken under GIT_TEST_PROTOCOL_VERSION=1
- tests: add a special setup where for protocol.version
- tests: add a check for unportable env --unset
This is (just a part of) v2, and I see v3 posted on the list, but
it seems that v3 needs further updates.
cf. <20181218124852.GD30471@sigill.intra.peff.net>
I'll drop the whole thing and wait for an update.
* jn/stripspace-wo-repository (2018-12-26) 1 commit
- stripspace: allow -s/-c outside git repository
"git stripspace" should be usable outside a git repository, but
under the "-s" or "-c" mode, it didn't.
Will merge to 'next'.
* ma/asciidoctor (2018-12-26) 3 commits
- git-status.txt: render tables correctly under Asciidoctor
- Documentation: do not nest open blocks
- git-column.txt: fix section header
Some of the documentation pages formatted incorrectly with
Asciidoctor, which have been fixed.
Will merge to 'next'.
* nd/worktree-remove-with-uninitialized-submodules (2019-01-07) 1 commit
- worktree: allow to (re)move worktrees with uninitialized submodules
"git worktree remove" and "git worktree move" refused to work when
there is a submodule involved. This has been loosened to ignore
uninitialized submodules.
Will merge to 'next'.
* sb/submodule-unset-core-worktree-when-worktree-is-lost (2018-12-26) 4 commits
- submodule deinit: unset core.worktree
- submodule--helper: fix BUG message in ensure_core_worktree
- submodule: unset core.worktree if no working tree is present
- submodule update: add regression test with old style setups
The core.worktree setting in a submodule repository should not be
pointing at a directory when the submodule loses its working tree
(e.g. getting deinit'ed), but the code did not properly maintain
this invariant.
Will merge to 'next'.
* so/cherry-pick-always-allow-m1 (2019-01-07) 4 commits
- t3506: validate '-m 1 -ff' is now accepted for non-merge commits
- t3502: validate '-m 1' argument is now accepted for non-merge commits
- cherry-pick: do not error on non-merge commits when '-m 1' is specified
- t3510: stop using '-m 1' to force failure mid-sequence of cherry-picks
"git cherry-pick -m1" was forbidden when picking a non-merge
commit, even though there _is_ parent number 1 for such a commit.
This was done to avoid mistakes back when "cherry-pick" was about
picking a single commit, but is no longer useful with "cherry-pick"
that can pick a range of commits. Now the "-m$num" option is
allowed when picking any commit, as long as $num names an existing
parent of the commit.
Technically this is a backward incompatible change; hopefully
nobody is relying on the error-checking behaviour.
Will merge to 'next'.
* cy/completion-typofix (2019-01-03) 1 commit
- completion: fix typo in git-completion.bash
Typofix.
Will merge to 'next'.
* cy/zsh-completion-SP-in-path (2019-01-03) 2 commits
- completion: treat results of git ls-tree as file paths
- zsh: complete unquoted paths with spaces correctly
With zsh, "git cmd path<TAB>" was completed to "git cmd path name"
when the completed path has a special character like SP in it,
without any attempt to keep "path name" a single filename. This
has been fixed to complete it to "git cmd path\ name" just like
Bash completion does.
Will merge to 'next'.
* ds/commit-graph-assert-missing-parents (2019-01-02) 1 commit
- commit-graph: writing missing parents is a BUG
Tightening error checking in commit-graph writer.
Will merge to 'next'.
* ed/simplify-setup-git-dir (2019-01-03) 1 commit
- Simplify handling of setup_git_directory_gently() failure cases.
Code simplification.
Will merge to 'next'.
* es/doc-worktree-guessremote-config (2018-12-28) 1 commit
- doc/config: do a better job of introducing 'worktree.guessRemote'
Doc clarification.
Will merge to 'next'.
* ew/ban-strncat (2019-01-02) 1 commit
- banned.h: mark strncat() as banned
The "strncat()" function is now among the banned functions.
Will merge to 'next'.
* jk/dev-build-format-security (2019-01-07) 1 commit
- config.mak.dev: add -Wall, primarily for -Wformat, to help autoconf users
Earlier we added "-Wformat-security" to developer builds, assuming
that "-Wall" (which includes "-Wformat" which in turn is required
to use "-Wformat-security") is always in effect. This is not true
when config.mak.autogen is in use, unfortunately. This has been
fixed by unconditionally adding "-Wall" to developer builds.
Will merge to 'next'.
* jp/author-committer-config (2019-01-02) 2 commits
- DONTMERGE
- Add author and committer configuration settings
Four new configuration variables {author,committer}.{name,email}
have been introduced to override user.{name,email} in more specific
cases.
Expecting a reroll.
cf. <xmqq1s5uk6qh.fsf@gitster-ct.c.googlers.com>
* js/rebase-am (2019-01-03) 4 commits
- built-in rebase: call `git am` directly
- rebase: teach `reset_head()` to optionally skip the worktree
- rebase: avoid double reflog entry when switching branches
- rebase: move `reset_head()` into a better spot
Instead of going through "git-rebase--am" scriptlet to use the "am"
backend, the built-in version of "git rebase" learned to drive the
"am" backend directly.
Waiting for a review response.
Looked almost ready.
* ms/packet-err-check (2019-01-02) 2 commits
- pack-protocol.txt: accept error packets in any context
- Use packet_reader instead of packet_read_line
Error checking of data sent over the pack-protocol has been
revamped so that error packets are always diagnosed properly.
* os/rebase-runs-post-checkout-hook (2019-01-02) 2 commits
- rebase: run post-checkout hook on checkout
- t5403: simplify by using a single repository
"git rebase" internally runs "checkout" to switch between branches,
and the command used to call the post-checkout hook, but the
reimplementation stopped doing so, which is getting fixed.
* rb/hpe (2019-01-03) 5 commits
- compat/regex/regcomp.c: define intptr_t and uintptr_t on NonStop
- git-compat-util.h: add FLOSS headers for HPE NonStop
- config.mak.uname: support for modern HPE NonStop config.
- transport-helper: drop read/write errno checks
- transport-helper: use xread instead of read
Portability updates for the HPE NonStop platform.
Will merge to 'next'.
* sg/test-bash-version-fix (2019-01-03) 2 commits
- Merge branch 'sg/test-bash-version-fix'
- test-lib: check Bash version for '-x' without using shell arrays
(this branch is used by sg/stress-test.)
The test suite tried to see if it is run under bash, but the check
itself failed under some other implementations of shell (notably
under NetBSD). This has been corrected.
Will merge to 'next'.
* sm/http-no-more-failonerror (2019-01-03) 2 commits
- Unset CURLOPT_FAILONERROR
- Change how HTTP response body is returned
Waiting for clarifications.
* tg/t5570-drop-racy-test (2019-01-07) 2 commits
- Revert "t/lib-git-daemon: record daemon log"
- t5570: drop racy test
An inherently racy test that caused intermittent failures has been
removed.
Will merge to 'next'.
* tt/bisect-in-c (2019-01-02) 7 commits
- bisect--helper: `bisect_start` shell function partially in C
- bisect--helper: `get_terms` & `bisect_terms` shell function in C
- bisect--helper: `bisect_next_check` shell function in C
- bisect--helper: `check_and_set_terms` shell function in C
- wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()
- bisect--helper: `bisect_write` shell function in C
- bisect--helper: `bisect_reset` shell function in C
More code in "git bisect" has been rewritten in C.
* ja/doc-build-l10n (2019-01-07) 1 commit
- Documentation/Makefile add optional targets for l10n
Prepare Documentation/Makefile so that manpage localization can
reuse it by overriding and tweaking the list of build products.
Will merge to 'next'.
* jk/loose-object-cache-oid (2019-01-07) 5 commits
- sha1-file: modernize loose header/stream functions
- sha1-file: modernize loose object file functions
- http: use struct object_id instead of bare sha1
- update comment references to sha1_object_info()
- sha1-file: fix outdated sha1 comment references
(this branch uses rs/loose-object-cache-perffix.)
Code clean-up.
Will merge to 'next'.
* mm/multimail-1.5 (2019-01-07) 1 commit
- git-multimail: update to release 1.5.0
Update "git multimail" from the upstream.
Will merge to 'next'.
* po/git-p4-wo-login (2019-01-07) 1 commit
- git-p4: fix problem when p4 login is not necessary
"git p4" update.
Will merge to 'next'.
* rs/loose-object-cache-perffix (2019-01-07) 4 commits
- object-store: retire odb_load_loose_cache()
- object-store: use one oid_array per subdirectory for loose cache
- object-store: factor out odb_clear_loose_cache()
- object-store: factor out odb_loose_cache()
(this branch is used by jk/loose-object-cache-oid.)
The loose object cache used to optimize existence look-up has been
updated.
Will merge to 'next'.
* rs/sha1-file-close-mapped-file-on-error (2019-01-07) 1 commit
- sha1-file: close fd of empty file in map_sha1_file_1()
Code clean-up.
Will merge to 'next'.
--------------------------------------------------
[Cooking]
* lt/date-human (2019-01-02) 3 commits
- t0006-date.sh: add `human` date format tests.
- Add 'human' date format documentation
- Add 'human' date format
A new date format "--date=human" that morphs its output depending
on how far the time is from the current time has been introduced.
"--date=auto" can be used to use this new format when the output is
goint to the pager or to the terminal and otherwise the default
format.
The design around "auto" may need to be rethought.
The tests need to be updated, too.
cf. <20190104075034.GA26014@sigill.intra.peff.net>
cf. <a5412274-028f-3662-e4f5-dbbcad4d9a40@iee.org>
* ds/midx-expire-repack (2019-01-02) 7 commits
- midx: implement midx_repack()
- multi-pack-index: prepare 'repack' subcommand
- multi-pack-index: implement 'expire' verb
- midx: refactor permutation logic
- multi-pack-index: prepare for 'expire' subcommand
- Docs: rearrange subcommands for multi-pack-index
- repack: refactor pack deletion for future use
* ds/push-sparse-tree-walk (2018-12-11) 6 commits
- pack-objects: create GIT_TEST_PACK_SPARSE
- pack-objects: create pack.useSparse setting
- revision: implement sparse algorithm
- pack-objects: add --sparse option
- list-objects: consume sparse tree walk
- revision: add mark_tree_uninteresting_sparse
* js/rebase-i-redo-exec (2018-12-11) 3 commits
- rebase: introduce a shortcut for --reschedule-failed-exec
- rebase: add a config option to default to --reschedule-failed-exec
- rebase: introduce --reschedule-failed-exec
* md/list-objects-filter-by-depth (2018-12-28) 4 commits
- tree:<depth>: skip some trees even when collecting omits
- list-objects-filter: teach tree:# how to handle >0
- Merge branch 'nd/the-index' into md/list-objects-filter-by-depth
- Merge branch 'sb/more-repo-in-api' into md/list-objects-filter-by-depth
(this branch uses sb/more-repo-in-api; is tangled with jt/get-reference-with-commit-graph.)
* nd/backup-log (2018-12-10) 24 commits
- FIXME
- rebase: keep backup of overwritten files on --skip or --abort
- am: keep backup of overwritten files on --skip or --abort
- checkout -f: keep backup of overwritten files
- reset --hard: keep backup of overwritten files
- unpack-trees.c: keep backup of ignored files being overwritten
- refs: keep backup of deleted reflog
- config --edit: support backup log
- sha1-file.c: let index_path() accept NULL istate
- backup-log: keep all blob references around
- gc: prune backup logs
- backup-log: add prune command
- backup-log: add log command
- backup-log: add diff command
- backup-log: add cat command
- backup-log.c: add API for walking backup log
- add--interactive: support backup log
- apply: support backup log with --keep-backup
- commit: support backup log
- update-index: support backup log with --keep-backup
- add: support backup log
- read-cache.c: new flag for add_index_entry() to write to backup log
- backup-log: add "update" subcommand
- doc: introduce new "backup log" concept
* nd/style-opening-brace (2018-12-10) 1 commit
- style: the opening '{' of a function is in a separate line
Code clean-up.
Will merge to 'next'.
* sg/stress-test (2019-01-07) 8 commits
- test-lib: add the '--stress' option to run a test repeatedly under load
- test-lib-functions: introduce the 'test_set_port' helper function
- test-lib: set $TRASH_DIRECTORY earlier
- test-lib: consolidate naming of test-results paths
- test-lib: parse command line options earlier
- test-lib: parse options in a for loop to keep $@ intact
- test-lib: extract Bash version check for '-x' tracing
- test-lib: translate SIGTERM and SIGHUP to an exit
(this branch uses sg/test-bash-version-fix.)
Flaky tests can now be repeatedly run under load with the
"--stress" option.
Will merge to 'next'.
* tg/checkout-no-overlay (2019-01-02) 8 commits
- checkout: introduce checkout.overlayMode config
- checkout: introduce --{,no-}overlay option
- checkout: factor out mark_cache_entry_for_checkout function
- checkout: clarify comment
- read-cache: add invalidate parameter to remove_marked_cache_entries
- entry: support CE_WT_REMOVE flag in checkout_entry
- entry: factor out unlink_entry function
- move worktree tests to t24*
"git checkout --no-overlay" can be used to trigger a new mode of
checking out paths out of the tree-ish, that allows paths that
match the pathspec that are in the current index and working tree
and are not in the tree-ish.
Will merge to 'next'.
* jk/proto-v2-hidden-refs-fix (2018-12-14) 3 commits
- upload-pack: support hidden refs with protocol v2
- parse_hide_refs_config: handle NULL section
- serve: pass "config context" through to individual commands
The v2 upload-pack protocol implementation failed to honor
hidden-ref configuration, which has been corrected.
Will merge to 'next'.
* la/quiltimport-keep-non-patch (2018-12-14) 1 commit
- git-quiltimport: Add --keep-non-patch option
"git quiltimport" learned "--keep-non-patch" option.
Will merge to 'next'.
* sb/submodule-fetchjobs-default-to-one (2018-12-14) 1 commit
- submodule update: run at most one fetch job unless otherwise set
"git submodule update" ought to use a single job unless asked, but
by mistake used multiple jobs, which has been fixed.
Will merge to 'next'.
* cb/openbsd-allows-reading-directory (2018-12-03) 1 commit
(merged to 'next' on 2019-01-04 at 9d865107fd)
+ config.mak.uname: OpenBSD uses BSD semantics with fread for directories
BSD port update.
Will merge to 'master'.
* cb/t5004-empty-tar-archive-fix (2018-12-03) 1 commit
(merged to 'next' on 2019-01-04 at 39f4cf94ce)
+ t5004: avoid using tar for empty packages
BSD port update.
Will merge to 'master'.
* cb/test-lint-cp-a (2018-12-03) 1 commit
(merged to 'next' on 2019-01-04 at d13e6cfcb0)
+ tests: add lint for non portable cp -a
BSD port update.
Will merge to 'master'.
* hb/t0061-dot-in-path-fix (2018-12-03) 1 commit
(merged to 'next' on 2019-01-04 at 789f990c4e)
+ t0061: do not fail test if '.' is part of $PATH
Test update.
Will merge to 'master'.
* hn/highlight-sideband-keywords (2018-12-04) 1 commit
(merged to 'next' on 2019-01-04 at b039601533)
+ sideband: color lines with keyword only
Lines that begin with a certain keyword that come over the wire, as
well as lines that consist only of one of these keywords, ought to
be painted in color for easier eyeballing, but the latter was
broken ever since the feature was introduced in 2.19, which has
been corrected.
Will merge to 'master'.
* js/commit-graph-chunk-table-fix (2018-12-14) 3 commits
- Makefile: correct example fuzz build
- commit-graph: fix buffer read-overflow
- commit-graph, fuzz: add fuzzer for commit-graph
The codepath to read from the commit-graph file attempted to read
past the end of it when the file's table-of-contents was corrupt.
* jt/get-reference-with-commit-graph (2018-12-28) 1 commit
- revision: use commit graph in get_reference()
(this branch uses sb/more-repo-in-api; is tangled with md/list-objects-filter-by-depth.)
Micro-optimize the code that prepares commit objects to be walked
by "git rev-list" when the commit-graph is available.
Needs to be rebuilt when sb/more-repo-in-api is rewound.
* md/exclude-promisor-objects-fix-cleanup (2018-12-06) 1 commit
(merged to 'next' on 2019-01-04 at c15362832d)
+ revision.c: put promisor option in specialized struct
Code clean-up.
Will merge to 'master'.
* bw/mailmap (2018-12-09) 1 commit
(merged to 'next' on 2019-01-04 at 02b6e83231)
+ mailmap: update brandon williams's email address
Will merge to 'master'.
* do/gitweb-strict-export-conf-doc (2018-12-09) 1 commit
(merged to 'next' on 2019-01-04 at 5249c9386a)
+ docs: fix $strict_export text in gitweb.conf.txt
Doc update.
Will merge to 'master'.
* en/directory-renames-nothanks-doc-update (2018-12-09) 1 commit
(merged to 'next' on 2019-01-04 at cb7134b54c)
+ git-rebase.txt: update note about directory rename detection and am
Doc update.
Will merge to 'master'.
* fd/gitweb-snapshot-conf-doc-fix (2018-12-09) 1 commit
(merged to 'next' on 2019-01-04 at 7ba71fca17)
+ docs/gitweb.conf: config variable typo
Doc update.
Will merge to 'master'.
* km/rebase-doc-typofix (2018-12-10) 1 commit
(merged to 'next' on 2019-01-04 at c89f646e8f)
+ rebase docs: drop stray word in merge command description
Doc update.
Will merge to 'master'.
* nd/indentation-fix (2018-12-09) 1 commit
(merged to 'next' on 2019-01-04 at 738b17d365)
+ Indent code with TABs
Code cleanup.
Will merge to 'master'.
* tb/use-common-win32-pathfuncs-on-cygwin (2018-12-26) 1 commit
(merged to 'next' on 2019-01-04 at c3b2b1f3c3)
+ git clone <url> C:\cygwin\home\USER\repo' is working (again)
Cygwin update.
Will merge to 'master'.
* tb/log-G-binary (2018-12-26) 1 commit
(merged to 'next' on 2019-01-04 at a713ef389c)
+ log -G: ignore binary files
"git log -G<regex>" looked for a hunk in the "git log -p" patch
output that contained a string that matches the given pattern.
Optimize this code to ignore binary files, which by default will
not show any hunk that would match any pattern (unless textconv or
the --text option is in effect, that is).
Will merge to 'master'.
* dl/merge-cleanup-scissors-fix (2018-11-21) 2 commits
- merge: add scissors line on merge conflict
- t7600: clean up 'merge --squash c3 with c7' test
The list of conflicted paths shown in the editor while concluding a
conflicted merge was shown above the scissors line when the
clean-up mode is set to "scissors", even though it was commented
out just like the list of updated paths and other information to
help the user explain the merge better.
Kicked out of 'next', to replace with a newer iteration.
cf. <cover.1545745331.git.liu.denton@gmail.com>
* aw/pretty-trailers (2018-12-09) 7 commits
- pretty: add support for separator option in %(trailers)
- strbuf: separate callback for strbuf_expand:ing literals
- pretty: add support for "valueonly" option in %(trailers)
- pretty: allow showing specific trailers
- pretty: single return path in %(trailers) handling
- pretty: allow %(trailers) options with explicit value
- doc: group pretty-format.txt placeholders descriptions
The %(trailers) formatter in "git log --format=..." now allows to
optionally pick trailers selectively by keyword, show only values,
etc.
How's the doneness of this one?
* nd/attr-pathspec-in-tree-walk (2018-11-19) 5 commits
(merged to 'next' on 2019-01-04 at 6a07e5b905)
+ tree-walk: support :(attr) matching
+ dir.c: move, rename and export match_attrs()
+ pathspec.h: clean up "extern" in function declarations
+ tree-walk.c: make tree_entry_interesting() take an index
+ tree.c: make read_tree*() take 'struct repository *'
The traversal over tree objects has learned to honor
":(attr:label)" pathspec match, which has been implemented only for
enumerating paths on the filesystem.
Will merge to 'master'.
* ab/commit-graph-progress-fix (2018-11-20) 1 commit
(merged to 'next' on 2019-01-04 at 405a1a2cf5)
+ commit-graph: split up close_reachable() progress output
Will merge to 'master'.
* jn/unknown-index-extensions (2018-11-21) 2 commits
- index: offer advice for unknown index extensions
- index: do not warn about unrecognized extensions
A bit too alarming warning given when unknown index extensions
exist is getting revamped.
Expecting a reroll.
* nd/checkout-noisy (2018-11-20) 2 commits
(merged to 'next' on 2019-01-04 at 480172d3d7)
+ t0027: squelch checkout path run outside test_expect_* block
+ checkout: print something when checking out paths
"git checkout [<tree-ish>] path..." learned to report the number of
paths that have been checked out of the index or the tree-ish,
which gives it the same degree of noisy-ness as the case in which
the command checks out a branch.
Will merge to 'master'.
* en/rebase-merge-on-sequencer (2019-01-07) 8 commits
- rebase: implement --merge via the interactive machinery
- rebase: define linearization ordering and enforce it
- git-legacy-rebase: simplify unnecessary triply-nested if
- git-rebase, sequencer: extend --quiet option for the interactive machinery
- am, rebase--merge: do not overlook --skip'ed commits with post-rewrite
- t5407: add a test demonstrating how interactive handles --skip differently
- rebase: fix incompatible options error message
- rebase: make builtin and legacy script error messages the same
"git rebase --merge" as been reimplemented by reusing the internal
machinery used for "git rebase -i".
* dl/remote-save-to-push (2018-12-11) 1 commit
- remote: add --save-to-push option to git remote set-url
"git remote set-url" learned a new option that moves existing value
of the URL field to pushURL field of the remote before replacing
the URL field with a new value.
I am personally not yet quite convinced if this is worth pursuing.
* js/protocol-advertise-multi (2018-12-28) 1 commit
- protocol: advertise multiple supported versions
The transport layer has been updated so that the protocol version
used can be negotiated between the parties, by the initiator
listing the protocol versions it is willing to talk, and the other
side choosing from one of them.
* js/smart-http-detect-remote-error (2019-01-07) 3 commits
- remote-curl: die on server-side errors
- remote-curl: tighten "version 2" check for smart-http
- remote-curl: refactor smart-http discovery
Some errors from the other side coming over smart HTTP transport
were not noticed, which has been corrected.
* nb/branch-show-other-worktrees-head (2019-01-07) 4 commits
- branch: add an extra verbose output displaying worktree path for checked out branch
- branch: mark and color a branch that is checked out in a linked worktree differently
- SQUASH???
- ref-filter: add worktreepath atom
"git branch --list" learned to show branches that are checked out
in other worktrees connected to the same repository prefixed with
'+', similar to the way the currently checked out branch is shown
with '*' in front.
* ot/ref-filter-object-info (2018-12-28) 6 commits
- ref-filter: add docs for new options
- ref-filter: add tests for deltabase
- ref-filter: add deltabase option
- ref-filter: add tests for objectsize:disk
- ref-filter: add check for negative file size
- ref-filter: add objectsize:disk option
The "--format=<placeholder>" option of for-each-ref, branch and tag
learned to show a few more traits of objects that can be learned by
the object_info API.
Will merge to 'next'.
* sb/diff-color-moved-config-option-fixup (2018-11-14) 1 commit
(merged to 'next' on 2019-01-04 at 46de5f42d1)
+ diff: align move detection error handling with other options
Minor inconsistency fix.
Will merge to 'master'.
* md/list-lazy-objects-fix (2018-12-06) 1 commit
(merged to 'next' on 2019-01-04 at 93bd38fff9)
+ list-objects.c: don't segfault for missing cmdline objects
"git rev-list --exclude-promisor-objects" had to take an object
that does not exist locally (and is lazily available) from the
command line without barfing, but the code dereferenced NULL.
Will merge to 'master'.
* sb/more-repo-in-api (2018-12-28) 23 commits
- t/helper/test-repository: celebrate independence from the_repository
- path.h: make REPO_GIT_PATH_FUNC repository agnostic
- commit: prepare free_commit_buffer and release_commit_memory for any repo
- commit-graph: convert remaining functions to handle any repo
- submodule: don't add submodule as odb for push
- submodule: use submodule repos for object lookup
- pretty: prepare format_commit_message to handle arbitrary repositories
- commit: prepare logmsg_reencode to handle arbitrary repositories
- commit: prepare repo_unuse_commit_buffer to handle any repo
- commit: prepare get_commit_buffer to handle any repo
- commit-reach: prepare in_merge_bases[_many] to handle any repo
- commit-reach: prepare get_merge_bases to handle any repo
- commit-reach.c: allow get_merge_bases_many_0 to handle any repo
- commit-reach.c: allow remove_redundant to handle any repo
- commit-reach.c: allow merge_bases_many to handle any repo
- commit-reach.c: allow paint_down_to_common to handle any repo
- commit: allow parse_commit* to handle any repo
- object: parse_object to honor its repository argument
- object-store: prepare has_{sha1, object}_file to handle any repo
- object-store: prepare read_object_file to deal with any repo
- object-store: allow read_object_file_extended to read from any repo
- packfile: allow has_packed_and_bad to handle arbitrary repositories
- sha1_file: allow read_object to read objects in arbitrary repositories
(this branch is used by jt/get-reference-with-commit-graph and md/list-objects-filter-by-depth.)
The in-core repository instances are passed through more codepaths.
* bc/sha-256 (2018-11-14) 12 commits
- hash: add an SHA-256 implementation using OpenSSL
- sha256: add an SHA-256 implementation using libgcrypt
- Add a base implementation of SHA-256 support
- commit-graph: convert to using the_hash_algo
- t/helper: add a test helper to compute hash speed
- sha1-file: add a constant for hash block size
- t: make the sha1 test-tool helper generic
- t: add basic tests for our SHA-1 implementation
- cache: make hashcmp and hasheq work with larger hashes
- hex: introduce functions to print arbitrary hashes
- sha1-file: provide functions to look up hash algorithms
- sha1-file: rename algorithm to "sha1"
Add sha-256 hash and plug it through the code to allow building Git
with the "NewHash".
* js/vsts-ci (2018-10-16) 13 commits
. travis: fix skipping tagged releases
. README: add a build badge (status of the Azure Pipelines build)
. tests: record more stderr with --write-junit-xml in case of failure
. tests: include detailed trace logs with --write-junit-xml upon failure
. git-p4: use `test_atexit` to kill the daemon
. git-daemon: use `test_atexit` in the tests
. tests: introduce `test_atexit`
. ci: add a build definition for Azure DevOps
. ci/lib.sh: add support for Azure Pipelines
. tests: optionally write results as JUnit-style .xml
. test-date: add a subcommand to measure times in shell scripts
. ci/lib.sh: encapsulate Travis-specific things
. ci: rename the library of common functions
Prepare to run test suite on Azure DevOps.
Ejected out of 'pu', as doing so seems to help other topics get
tested at TravisCI.
https://travis-ci.org/git/git/builds/452713184 is a sample of a
build whose tests on 4 hang (with this series in). Ejecting it
gave us https://travis-ci.org/git/git/builds/452778963 which still
shows breakages from other topics not yet in 'next', but at least
the tests do not stall.
* du/branch-show-current (2018-10-26) 1 commit
- branch: introduce --show-current display option
"git branch" learned a new subcommand "--show-current".
I am personally not yet quite convinced if this is worth pursuing.
* mk/use-size-t-in-zlib (2018-10-15) 1 commit
- zlib.c: use size_t for size
The wrapper to call into zlib followed our long tradition to use
"unsigned long" for sizes of regions in memory, which have been
updated to use "size_t".
* ag/sequencer-reduce-rewriting-todo (2018-11-12) 16 commits
. rebase--interactive: move transform_todo_file() to rebase--interactive.c
. sequencer: fix a call to error() in transform_todo_file()
. sequencer: use edit_todo_list() in complete_action()
. rebase-interactive: rewrite edit_todo_list() to handle the initial edit
. rebase-interactive: append_todo_help() changes
. rebase-interactive: use todo_list_write_to_file() in edit_todo_list()
. sequencer: refactor skip_unnecessary_picks() to work on a todo_list
. sequencer: change complete_action() to use the refactored functions
. sequencer: make sequencer_make_script() write its script to a strbuf
. sequencer: refactor rearrange_squash() to work on a todo_list
. sequencer: refactor sequencer_add_exec_commands() to work on a todo_list
. sequencer: refactor check_todo_list() to work on a todo_list
. sequencer: introduce todo_list_write_to_file()
. sequencer: refactor transform_todos() to work on a todo_list
. sequencer: make the todo_list structure public
. sequencer: changes in parse_insn_buffer()
The scripted version of "git rebase -i" wrote and rewrote the todo
list many times during a single step of its operation, and the
recent C-rewrite made a faithful conversion of the logic to C. The
implementation has been updated to carry necessary information
around in-core to avoid rewriting the same file over and over
unnecessarily.
With too many topics in-flight that touch sequencer and rebaser,
this need to wait giving precedence to other topics that fix bugs.
* sb/submodule-recursive-fetch-gets-the-tip (2018-12-09) 9 commits
- fetch: ensure submodule objects fetched
- submodule.c: fetch in submodules git directory instead of in worktree
- submodule: migrate get_next_submodule to use repository structs
- repository: repo_submodule_init to take a submodule struct
- submodule: store OIDs in changed_submodule_names
- submodule.c: tighten scope of changed_submodule_names struct
- submodule.c: sort changed_submodule_names before searching it
- submodule.c: fix indentation
- sha1-array: provide oid_array_filter
"git fetch --recurse-submodules" may not fetch the necessary commit
that is bound to the superproject, which is getting corrected.
Ready?
* js/add-i-coalesce-after-editing-hunk (2018-08-28) 1 commit
- add -p: coalesce hunks before testing applicability
Applicability check after a patch is edited in a "git add -i/p"
session has been improved.
Will hold.
cf. <e5b2900a-0558-d3bf-8ea1-d526b078bbc2@talktalk.net>
* ps/stash-in-c (2019-01-04) 27 commits
- tests: add a special setup where stash.useBuiltin is off
- stash: optionally use the scripted version again
- stash: add back the original, scripted `git stash`
- stash: convert `stash--helper.c` into `stash.c`
- stash: replace all `write-tree` child processes with API calls
- stash: optimize `get_untracked_files()` and `check_changes()`
- stash: convert save to builtin
- stash: make push -q quiet
- stash: convert push to builtin
- stash: convert create to builtin
- stash: convert store to builtin
- stash: convert show to builtin
- stash: convert list to builtin
- stash: convert pop to builtin
- stash: convert branch to builtin
- stash: convert drop and clear to builtin
- stash: convert apply to builtin
- stash: mention options in `show` synopsis
- stash: add tests for `git stash show` config
- stash: rename test cases to be more descriptive
- t3903: modernize style
- stash: improve option parsing test coverage
- ident: add the ability to provide a "fallback identity"
- strbuf.c: add `strbuf_insertf()` and `strbuf_vinsertf()`
- strbuf.c: add `strbuf_join_argv()`
- sha1-name.c: add `get_oidf()` which acts like `get_oid()`
- Merge branch 'sd/stash-wo-user-name'
"git stash" rewritten in C.
* pw/add-p-select (2018-07-26) 4 commits
- add -p: optimize line selection for short hunks
- add -p: allow line selection to be inverted
- add -p: select modified lines correctly
- add -p: select individual hunk lines
"git add -p" interactive interface learned to let users choose
individual added/removed lines to be used in the operation, instead
of accepting or rejecting a whole hunk.
Will discard.
No further feedbacks on the topic for quite some time.
cf. <d622a95b-7302-43d4-4ec9-b2cf3388c653@talktalk.net>
I found the feature to be hard to explain, and may result in more
end-user complaints, but let's see.
--------------------------------------------------
[Discarded]
* ab/reject-alias-loop (2018-10-19) 1 commit
. alias: detect loops in mixed execution mode
Two (or more) aliases that mutually refer to each other can form an
infinite loop; we now attempt to notice and stop.
Discarded.
Reverted out of 'next'.
cf. <87sh0slvxm.fsf@evledraar.gmail.com>
* gl/bundle-unlock-before-aborting (2018-11-14) 1 commit
. bundle: rollback lock file while refusing to create an empty bundle
Superseded by jk/close-duped-fd-before-unlock-for-bundle
* js/remote-archive-v2 (2018-09-28) 4 commits
. archive: allow archive over HTTP(S) with proto v2
. archive: implement protocol v2 archive command
. archive: use packet_reader for communications
. archive: follow test standards around assertions
The original implementation of "git archive --remote" more or less
bypassed the transport layer and did not work over http(s). The
version 2 of the protocol is defined to allow going over http(s) as
well as Git native transport.
Retracted; reverted out of next.
cf. <20181114195142.GI126896@google.com>
* ab/format-patch-rangediff-not-stat (2018-11-24) 1 commit
. format-patch: don't include --stat with --range-diff output
The "--rangediff" option recently added to "format-patch"
interspersed a bogus and useless "--stat" information by mistake,
which is being corrected.
Reverted out of 'next'.
* jc/postpone-rebase-in-c (2018-11-26) 1 commit
. rebase: mark the C reimplementation as an experimental opt-in feature
People seem to be still finding latent bugs in the "rebase in C"
reimplementation. For the upcoming release, use the scripted
version by default and adjust the documentation accordingly.
Reverted out of 'next'.
^ permalink raw reply
* Re: [PATCH v2 2/2] Unset CURLOPT_FAILONERROR
From: Masaya Suzuki @ 2019-01-07 23:24 UTC (permalink / raw)
To: Jeff King; +Cc: Git Mailing List, Jonathan Nieder, Eric Sunshine
In-Reply-To: <20190104104907.GC26185@sigill.intra.peff.net>
On Fri, Jan 4, 2019 at 2:49 AM Jeff King <peff@peff.net> wrote:
>
> On Sat, Dec 29, 2018 at 11:44:47AM -0800, Masaya Suzuki wrote:
>
> > 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.
>
> Out of curiosity, does GIT_TRACE_CURL do any better? Or is it simply
> that curl closes the handle when it sees the bad response code, and
> nobody ever gets to see the rest of the data?
curl disregards the rest of the contents when it sees a bad response
code when CURLOPT_FAILONERROR is set
(https://github.com/curl/curl/blob/dea3f94298ac0859464768959488938c4e104545/lib/http.c#L3691).
So nobody gets the rest of the data.
>
> > This is substantially same as setting http_options.keep_error to all
> > requests. Hence, remove this option.
>
> The assumption here is that every code path using FAILONERROR is
> prepared to handle the failing http response codes itself (since we no
> longer set it at all in get_active_slot()). Is that so?
I was thinking that I covered the all code paths using FAILONERROR,
but it seems it's not the case. http-walker.c and http-push.c also
calls get_active_slot(). I'll narrow down the scope on removing
FAILONERROR.
>
> Anything that uses handle_curl_result() is OK. That means run_one_slot()
> is fine, which in turn covers run_slot() for RPCs, and http_request()
> for normal one-at-a-time requests. But what about the parallel multiple
> requests issued by the dumb-http walker code?
Right. I'll keep FAILONERROR in get_active_slot and remove it only for
the code paths that can handle HTTP errors.
>
> There I think we end up in step_active_slots(), which calls into
> finish_active_slot() for completed requests. I think that
> unconditionally fetches the http code without bothering to look at
> whether curl reported success or not.
>
> So I _think_ that's probably all of the users of the curl handles
> provided by get_active_slot(). Though given the tangled mess of our HTTP
> code, I won't be surprised if there's a corner case I missed in that
> analysis.
>
> -Peff
^ permalink raw reply
* Re: cvs2git failed in pass2 ...
From: Jonathan Nieder @ 2019-01-07 23:13 UTC (permalink / raw)
To: paduarte; +Cc: git, users
In-Reply-To: <1546873568530-0.post@n2.nabble.com>
+users@cvs2svn.tigris.org
paduarte wrote:
Hi,
> How did the complete command work?
>
> cvs2git --encoding=ascii --encoding=utf8 --encoding=utf16 --encoding=latin
>
> or
>
> cvs2git --encoding=ascii --encoding=utf8 --encoding=utf16 --encoding=latin
> *+ parameters of git*
>
> ?
Cc-ing the cvs2git users list, who may be able to help you.
Thanks and hope that helps,
Jonathan
> Tks
> :)
^ permalink raw reply
* Re: [PATCH] blame: add the ability to ignore commits
From: Junio C Hamano @ 2019-01-07 23:13 UTC (permalink / raw)
To: Barret Rhoden; +Cc: git, Stefan Beller, Jeff Smith, Jeff King
In-Reply-To: <20190107213013.231514-1-brho@google.com>
Barret Rhoden <brho@google.com> writes:
> Commits that make formatting changes or renames are often not
> interesting when blaming a file. This commit, similar to
> git-hyper-blame, allows one to specify certain revisions to ignore during
> the blame process.
>
> To ignore a revision, put its committish in a file specified by
> --ignore-file=<file> or use -i <rev>, which can be repeated.
If I read it correctly, this gives a very limited form of -S, in the
sense that anything this can do can be expressed by using -S but the
reverse is not true, but is designed to be easier to use, in the
sense that unlike -S, this does not have to describe the part of the
history you do not have to lie about. The documentation should say
something about these pros-and-cons to help readers decide which
feature to use.
I somehow feel that this is rare enough that it should not squat on
short-and-sweet '-i'. We would want to reserve it to something like
"--ignore-case", for example.
> The file .git-blame-ignore-revs is checked by default.
Giving the projects a way to easily help participants to share the
same distorted view of the history is a good idea, but I do not
think we should allow projects to do so to those who merely clone it
without their consent. IOW, "by default" is a terrible idea.
Perhaps paying attention to blame.skipRevsFile configuration
variable that is set by the end user would be sufficient----the
project can ship .blame-skip-revs (or any random filename of their
choice) in-tree as tracked contents and tell its users that they can
optionally use it.
> It's useful to be alerted to the presence of an ignored commit in the
> history of a line. Those lines will be marked with '*' in the
> non-porcelain output. The '*' is attached to the line number to keep
> from breaking tools that rely on the whitespace between columns.
A policy decision like the above two shouldn't be hardcoded in the
feature like this, but should be done as a separate option. By
default, these shouldn't be marked with '*', as the same tools you
said you are afraid of breaking would be expecting a word with only
digits and no asterisk in the column where line numbers appear and
will get broken by this change if done unconditionally.
In general, I find this patch trying to change too many things at
the same time, without sufficient justification. Perhaps do these
different things as separate steps in a single series?
> A blame_entry attributed to an ignored commit will get passed to its
> parent.
Obviously, an interesting consideration is what happens when a merge
commit is skipped. Is it sufficient to change this description to
"...will get passed to its parentS", or would the code do completely
nonsensical things without further tweaks (a possible simple tweak
could be to refuse skipping merges)?
> If an ignored commit changed a line, an ancestor that changed
> the line will get blamed. However, if an ignored commit added lines, a
> commit changing a nearby line may get blamed. If no commit is found,
> the original commit for the file will get blamed.
The above somehow does not read as describing a carefully designed
behaviour; rather, it sounds as if it is saying "the code does
whatever random things it happens to do". For example, when there
is a newly added line how is "A" commit changing a nearby line
chosen (a line has lines before it and after it---they may be
touched by different commits, and before and after that skipped
commit, so there are multiple commits to choose from)?
> diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
> index 16323eb80e31..e41375374892 100644
> --- a/Documentation/git-blame.txt
> +++ b/Documentation/git-blame.txt
> @@ -10,6 +10,7 @@ SYNOPSIS
> [verse]
> 'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] [--incremental]
> [-L <range>] [-S <revs-file>] [-M] [-C] [-C] [-C] [--since=<date>]
> + [-i <rev>] [--no-default-ignores] [--ignore-file=<file>]
> [--progress] [--abbrev=<n>] [<rev> | --contents <file> | --reverse <rev>..<rev>]
> [--] <file>
>
> @@ -84,6 +85,20 @@ include::blame-options.txt[]
> Ignore whitespace when comparing the parent's version and
> the child's to find where the lines came from.
>
> +-i <rev>::
> + Ignore revision when assigning blame. Lines that were changed by an
> + ignored commit will be marked with a `*` in the blame output. Lines
> + that were added by an ignored commit may be attributed commits making
> + nearby changes or to the first commit touching the file.
It probably deserves to be told that this option can be given
multiple times and used cumulatively (unlike usual "last one wins"
rule).
> +--no-default-ignores::
> + Do not automatically ignore revisions in the file
> + `.git-blame-ignore-revs`.
This should not be "opt-out" like this.
> +--ignore-file=<file>::
> + Ignore revisions listed in `file`, one revision per line. Whitespace
> + and comments beginning with `#` are ignored.
Should it be capable to take two or more ignore-files? Or should we
use the usual "the last one wins" rule?
I think we should support blame.skipRevFile configuration variable
so that the users do not have to constantly give the option from the
command line. And with that, there is no need to have a hardcoded
filename .git-blame-ignore-revs or anything like that.
If we are to use configuration variable, however, we'd need a way to
override its use from the command line, too. Perhaps a sane
arrangement would be
- if one or more --skip-revs-file=<file> are given from the
command line, use all of them and ignore blame.skipRevsFile
configuration variable.
- if no --skip-revs-file=<file> is given from the command line,
use blame.skipRevsFile configuration variable.
- regardless of the above two, pay attention to --skip-rev=<rev>
command line option(s).
Somehow the damage to blame.c codebase looks way too noisy for what
the code wants to do. If all we want is to pretend in a history,
e.g.
---O---A---B---C---D
that commit B does not exist, i.e. use a distorted view of the
history
---O---A-------C---D
wouldn't it be sufficient to modify pass_blame(), i.e. the only the
caller of the pass_blame_to_parent(), where we find the parent
commits of "C" and dig the history to pass blame to "B", and have it
pretend as if "B" does not exist and pass blame directly to "A"?
Thanks. I am personally not all that interested in this yet.
^ permalink raw reply
* Re: [PATCH v2 2/2] pack-protocol.txt: accept error packets in any context
From: Josh Steadmon @ 2019-01-07 22:53 UTC (permalink / raw)
To: Masaya Suzuki; +Cc: git, peff
In-Reply-To: <20181229211915.161686-3-masayasuzuki@google.com>
On 2018.12.29 13:19, Masaya Suzuki wrote:
> 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
>
This all looks good to me.
Reviewed-by: Josh Steadmon <steadmon@google.com>
^ permalink raw reply
* Re: [PATCH v2 1/2] Use packet_reader instead of packet_read_line
From: Josh Steadmon @ 2019-01-07 22:33 UTC (permalink / raw)
To: Masaya Suzuki; +Cc: git, peff
In-Reply-To: <20181229211915.161686-2-masayasuzuki@google.com>
On 2018.12.29 13:19, Masaya Suzuki wrote:
> 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)
packet_read_line() can also return NULL if the packet is zero-length, so
you may want to add a "|| reader.pktlen <= 0" to the condition here (and
in other places where we were checking that packet_read_line() != NULL)
to make sure the behavior doesn't change. See discussion on my previous
attempt[1] to refactor this in builtin/archive.c.
[1]: https://public-inbox.org/git/20180912053519.31085-1-steadmon@google.com/
> 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)
And here you'd want to add "&& reader.pktlen > 0". I'll skip pointing
out further instances of this issue.
> 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) {
Similarly, here a delim packet would previously have been caught here as
well. So it might be better to just check "reader->pktlen == 0" rather
than looking at the status. But I see in the next "if" you're adding a
new check with a die(), so I guess you're not intending to preserve the
original behavior here.
> 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"));
Another behavior change here, as previously we didn't check for a final
flush packet (unless I'm missing something).
> }
> }
>
> -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
>
In general I think this looks good. If you want this to be a strict
refactor with no behavior changes, you'll want to address the comments
above. Otherwise, I'd prefer if you note in the commit message where/how
the behavior is changing.
^ permalink raw reply
* Re: Introducing OneDev - an open source git server with interesting features
From: Jonathan Nieder @ 2019-01-07 22:26 UTC (permalink / raw)
To: Robin Shen; +Cc: git@vger.kernel.org, jgit-dev
In-Reply-To: <SG2PR04MB3205E9C83085CFF2F426FE65B9890@SG2PR04MB3205.apcprd04.prod.outlook.com>
+jgit-dev
Hi Robin,
Robin Shen wrote:
> Dear git users,
>
> OneDev is an open source git server with interesting features such
> as language aware code search/navigation, issue workflow
> customization, free source/diff comment and discussion, etc.
>
> It is using MIT license and hope it can be useful to someone. Learn
> more at https://onedev.io
Thanks for writing! Looking at
https://github.com/theonedev/onedev/blob/master/core/pom.xml, it
appears this is a web interface that uses JGit for Git support.
Can you say a little about how it compares to Gitblit
<http://gitblit.com>, Phabricator, Gerrit, and other interfaces?
Also, if you have time for it, mind saying a little about what your
experience using JGit has been like? Any thoughts about what worked
well and what didn't work as well? This can help both the Git and
JGit projects:
- Git, since it can help us learn from JGit's successes and mistakes
- JGit, since it's not too late to make the API better :)
Sincerely,
Jonathan
^ permalink raw reply
* Re: [PATCH] git-p4: fix problem when p4 login is not necessary
From: Junio C Hamano @ 2019-01-07 22:24 UTC (permalink / raw)
To: Luke Diamand; +Cc: Peter Osterlund, Git Users
In-Reply-To: <CAE5ih7-qFrYXfsWr6MebSFBtDERLKGf0zb73yaV6Eit4PJHcJA@mail.gmail.com>
Luke Diamand <luke@diamand.org> writes:
> On Mon, 7 Jan 2019 at 20:51, Peter Osterlund <peterosterlund2@gmail.com> wrote:
>>
>> In a perforce setup where login is not required, communication fails
>> because p4_check_access does not understand the response from the p4
>> client. Fixed by detecting and ignoring the "info" response.
>
> This is caused by my earlier change in this area. I think this fix
> looks good, thanks.
> Ack.
>
>>
>> Signed-off-by: Peter Osterlund <peterosterlund2@gmail.com>
>> ---
>> git-p4.py | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/git-p4.py b/git-p4.py
>> index 1998c3e141..3e12774f96 100755
>> --- a/git-p4.py
>> +++ b/git-p4.py
>> @@ -332,6 +332,8 @@ def p4_check_access(min_expiration=1):
>> die_bad_access("p4 error: {0}".format(data))
>> else:
>> die_bad_access("unknown error")
>> + elif code == "info":
>> + return
>> else:
>> die_bad_access("unknown error code {0}".format(code))
>>
>>
>> --
>> Peter Osterlund - peterosterlund2@gmail.com
>> http://hem.bredband.net/petero2b
The patch was whitespace damaged, but for a two-liner like this, I
can type it myself instead, so no need to resend.
Thanks, both.
^ permalink raw reply
* Re: [RFC PATCH 1/1] filter-options: Expand abbreviated numbers
From: Junio C Hamano @ 2019-01-07 22:18 UTC (permalink / raw)
To: Josh Steadmon; +Cc: git
In-Reply-To: <xmqqva306rsy.fsf@gitster-ct.c.googlers.com>
Junio C Hamano <gitster@pobox.com> writes:
> Josh Steadmon <steadmon@google.com> writes:
>
>>> > - `rev-list` for possible "filter-spec" values.
>>> > + `rev-list` for possible "filter-spec" values. Clients MUST
>>> > + translate abbreviated numbers (e.g. "1k") into fully-expanded
>>> > + numbers (e.g. "1024") on the client side, so that the server
>>> > + does not need to implement unit parsing.
>>>
>>> I suspect that it is too late now to retroactively say "MUST" here.
>>> The best we may be able to do is to say "The sender SHOULD send a
>>> plain integer without unit and the receiver SHOULD be prepared to
>>> scale an integer with unit".
>>>
>>
>> In that case, do you think we should also specify that units should be
>> interpreted as powers-of-2 rather than powers-of-10?
>
> If we do not document the number scaling elsewhere, then we
> certainly should, but I somehow doubt it.
>
> Documentation/git-config.txt does list these explicitly where it
> talks about --type=int, but these human-readable scaled integers are
> also used in command line arguments as well, so we may want to find
> a central place that is clear to the readers that the description
> applies to all of them and move the description there.
Another thing. We may probably end up adding more scaling factors,
but going forward we would not want to require any and all Git
reimplementations to adopt them in lock-step, so perhaps
`rev-list` for possible "filter-spec" values. Senders SHOULD
translate a scaled integer (e.g. "1k") into a full expanded form
(e.g. "1024") so that an older receiver that does not know newly
invented scaling units can still interoperate with it, but
receivers SHOULD accept the following scalilng units: 'k', 'm'
and 'g' for 1024, 1048576 and 1073741824 respectively.
or something like that.
^ permalink raw reply
* Re: [RFC PATCH 1/1] filter-options: Expand abbreviated numbers
From: Junio C Hamano @ 2019-01-07 22:12 UTC (permalink / raw)
To: Josh Steadmon; +Cc: git
In-Reply-To: <20190107212517.GA54613@google.com>
Josh Steadmon <steadmon@google.com> writes:
>> > - `rev-list` for possible "filter-spec" values.
>> > + `rev-list` for possible "filter-spec" values. Clients MUST
>> > + translate abbreviated numbers (e.g. "1k") into fully-expanded
>> > + numbers (e.g. "1024") on the client side, so that the server
>> > + does not need to implement unit parsing.
>>
>> I suspect that it is too late now to retroactively say "MUST" here.
>> The best we may be able to do is to say "The sender SHOULD send a
>> plain integer without unit and the receiver SHOULD be prepared to
>> scale an integer with unit".
>>
>
> In that case, do you think we should also specify that units should be
> interpreted as powers-of-2 rather than powers-of-10?
If we do not document the number scaling elsewhere, then we
certainly should, but I somehow doubt it.
Documentation/git-config.txt does list these explicitly where it
talks about --type=int, but these human-readable scaled integers are
also used in command line arguments as well, so we may want to find
a central place that is clear to the readers that the description
applies to all of them and move the description there.
^ permalink raw reply
* Re: [PATCH] git-p4: fix problem when p4 login is not necessary
From: Luke Diamand @ 2019-01-07 22:05 UTC (permalink / raw)
To: Peter Osterlund; +Cc: Git Users, Junio C Hamano
In-Reply-To: <alpine.LFD.2.21.1901072148380.20807@fractal.localdomain>
On Mon, 7 Jan 2019 at 20:51, Peter Osterlund <peterosterlund2@gmail.com> wrote:
>
> In a perforce setup where login is not required, communication fails
> because p4_check_access does not understand the response from the p4
> client. Fixed by detecting and ignoring the "info" response.
This is caused by my earlier change in this area. I think this fix
looks good, thanks.
Ack.
>
> Signed-off-by: Peter Osterlund <peterosterlund2@gmail.com>
> ---
> git-p4.py | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/git-p4.py b/git-p4.py
> index 1998c3e141..3e12774f96 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -332,6 +332,8 @@ def p4_check_access(min_expiration=1):
> die_bad_access("p4 error: {0}".format(data))
> else:
> die_bad_access("unknown error")
> + elif code == "info":
> + return
> else:
> die_bad_access("unknown error code {0}".format(code))
>
>
> --
> Peter Osterlund - peterosterlund2@gmail.com
> http://hem.bredband.net/petero2b
^ permalink raw reply
* [PATCH] blame: add the ability to ignore commits
From: Barret Rhoden @ 2019-01-07 21:30 UTC (permalink / raw)
To: git; +Cc: Stefan Beller, Jeff Smith, Junio C Hamano, Jeff King
Commits that make formatting changes or renames are often not
interesting when blaming a file. This commit, similar to
git-hyper-blame, allows one to specify certain revisions to ignore during
the blame process.
To ignore a revision, put its committish in a file specified by
--ignore-file=<file> or use -i <rev>, which can be repeated. The file
.git-blame-ignore-revs is checked by default.
It's useful to be alerted to the presence of an ignored commit in the
history of a line. Those lines will be marked with '*' in the
non-porcelain output. The '*' is attached to the line number to keep
from breaking tools that rely on the whitespace between columns.
A blame_entry attributed to an ignored commit will get passed to its
parent. If an ignored commit changed a line, an ancestor that changed
the line will get blamed. However, if an ignored commit added lines, a
commit changing a nearby line may get blamed. If no commit is found,
the original commit for the file will get blamed.
Signed-off-by: Barret Rhoden <brho@google.com>
---
Documentation/git-blame.txt | 15 +++++++++
blame.c | 38 +++++++++++++++++----
blame.h | 3 ++
builtin/blame.c | 66 +++++++++++++++++++++++++++++++++++--
4 files changed, 112 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index 16323eb80e31..e41375374892 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -10,6 +10,7 @@ SYNOPSIS
[verse]
'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] [--incremental]
[-L <range>] [-S <revs-file>] [-M] [-C] [-C] [-C] [--since=<date>]
+ [-i <rev>] [--no-default-ignores] [--ignore-file=<file>]
[--progress] [--abbrev=<n>] [<rev> | --contents <file> | --reverse <rev>..<rev>]
[--] <file>
@@ -84,6 +85,20 @@ include::blame-options.txt[]
Ignore whitespace when comparing the parent's version and
the child's to find where the lines came from.
+-i <rev>::
+ Ignore revision when assigning blame. Lines that were changed by an
+ ignored commit will be marked with a `*` in the blame output. Lines
+ that were added by an ignored commit may be attributed commits making
+ nearby changes or to the first commit touching the file.
+
+--no-default-ignores::
+ Do not automatically ignore revisions in the file
+ `.git-blame-ignore-revs`.
+
+--ignore-file=<file>::
+ Ignore revisions listed in `file`, one revision per line. Whitespace
+ and comments beginning with `#` are ignored.
+
--abbrev=<n>::
Instead of using the default 7+1 hexadecimal digits as the
abbreviated object name, use <n>+1 digits. Note that 1 column
diff --git a/blame.c b/blame.c
index d84c93778080..9e338cfa83e3 100644
--- a/blame.c
+++ b/blame.c
@@ -474,7 +474,8 @@ void blame_coalesce(struct blame_scoreboard *sb)
for (ent = sb->ent; ent && (next = ent->next); ent = next) {
if (ent->suspect == next->suspect &&
- ent->s_lno + ent->num_lines == next->s_lno) {
+ ent->s_lno + ent->num_lines == next->s_lno &&
+ ent->ignored == next->ignored) {
ent->num_lines += next->num_lines;
ent->next = next->next;
blame_origin_decref(next->suspect);
@@ -726,6 +727,8 @@ static void split_overlap(struct blame_entry *split,
int chunk_end_lno;
memset(split, 0, sizeof(struct blame_entry [3]));
+ split[0].ignored = split[1].ignored = split[2].ignored = e->ignored;
+
if (e->s_lno < tlno) {
/* there is a pre-chunk part not blamed on parent */
split[0].suspect = blame_origin_incref(e->suspect);
@@ -845,10 +848,10 @@ static struct blame_entry *reverse_blame(struct blame_entry *head,
*/
static void blame_chunk(struct blame_entry ***dstq, struct blame_entry ***srcq,
int tlno, int offset, int same,
- struct blame_origin *parent)
+ struct blame_origin *parent, int ignore_diffs)
{
struct blame_entry *e = **srcq;
- struct blame_entry *samep = NULL, *diffp = NULL;
+ struct blame_entry *samep = NULL, *diffp = NULL, *ignoredp = NULL;
while (e && e->s_lno < tlno) {
struct blame_entry *next = e->next;
@@ -862,6 +865,7 @@ static void blame_chunk(struct blame_entry ***dstq, struct blame_entry ***srcq,
int len = tlno - e->s_lno;
struct blame_entry *n = xcalloc(1, sizeof (struct blame_entry));
n->suspect = e->suspect;
+ n->ignored = e->ignored;
n->lno = e->lno + len;
n->s_lno = e->s_lno + len;
n->num_lines = e->num_lines - len;
@@ -916,6 +920,7 @@ static void blame_chunk(struct blame_entry ***dstq, struct blame_entry ***srcq,
int len = same - e->s_lno;
struct blame_entry *n = xcalloc(1, sizeof (struct blame_entry));
n->suspect = blame_origin_incref(e->suspect);
+ n->ignored = e->ignored;
n->lno = e->lno + len;
n->s_lno = e->s_lno + len;
n->num_lines = e->num_lines - len;
@@ -925,10 +930,24 @@ static void blame_chunk(struct blame_entry ***dstq, struct blame_entry ***srcq,
n->next = samep;
samep = n;
}
- e->next = diffp;
- diffp = e;
+ if (ignore_diffs) {
+ /* These go to the parent, like the ones before tlno. */
+ blame_origin_decref(e->suspect);
+ e->suspect = blame_origin_incref(parent);
+ e->s_lno += offset;
+ e->ignored = 1;
+ e->next = ignoredp;
+ ignoredp = e;
+ } else {
+ e->next = diffp;
+ diffp = e;
+ }
e = next;
}
+ if (ignoredp) {
+ **dstq = reverse_blame(ignoredp, **dstq);
+ *dstq = &ignoredp->next;
+ }
**srcq = reverse_blame(diffp, reverse_blame(samep, e));
/* Move across elements that are in the unblamable portion */
if (diffp)
@@ -938,6 +957,7 @@ static void blame_chunk(struct blame_entry ***dstq, struct blame_entry ***srcq,
struct blame_chunk_cb_data {
struct blame_origin *parent;
long offset;
+ int ignore_diffs;
struct blame_entry **dstq;
struct blame_entry **srcq;
};
@@ -950,7 +970,7 @@ static int blame_chunk_cb(long start_a, long count_a,
if (start_a - start_b != d->offset)
die("internal error in blame::blame_chunk_cb");
blame_chunk(&d->dstq, &d->srcq, start_b, start_a - start_b,
- start_b + count_b, d->parent);
+ start_b + count_b, d->parent, d->ignore_diffs);
d->offset = start_a + count_a - (start_b + count_b);
return 0;
}
@@ -973,18 +993,22 @@ static void pass_blame_to_parent(struct blame_scoreboard *sb,
d.parent = parent;
d.offset = 0;
+ d.ignore_diffs = 0;
d.dstq = &newdest; d.srcq = &target->suspects;
fill_origin_blob(&sb->revs->diffopt, parent, &file_p, &sb->num_read_blob);
fill_origin_blob(&sb->revs->diffopt, target, &file_o, &sb->num_read_blob);
sb->num_get_patch++;
+ if (oidset_contains(&sb->ignores, &target->commit->object.oid))
+ d.ignore_diffs = 1;
+
if (diff_hunks(&file_p, &file_o, blame_chunk_cb, &d, sb->xdl_opts))
die("unable to generate diff (%s -> %s)",
oid_to_hex(&parent->commit->object.oid),
oid_to_hex(&target->commit->object.oid));
/* The rest are the same as the parent */
- blame_chunk(&d.dstq, &d.srcq, INT_MAX, d.offset, INT_MAX, parent);
+ blame_chunk(&d.dstq, &d.srcq, INT_MAX, d.offset, INT_MAX, parent, 0);
*d.dstq = NULL;
queue_blames(sb, parent, newdest);
diff --git a/blame.h b/blame.h
index be3a895043e0..3fe71a59d372 100644
--- a/blame.h
+++ b/blame.h
@@ -92,6 +92,7 @@ struct blame_entry {
* scanning the lines over and over.
*/
unsigned score;
+ int ignored;
};
/*
@@ -117,6 +118,8 @@ struct blame_scoreboard {
/* linked list of blames */
struct blame_entry *ent;
+ struct oidset ignores;
+
/* look-up a line in the final buffer */
int num_lines;
int *lineno;
diff --git a/builtin/blame.c b/builtin/blame.c
index 6d798f99392e..698834426771 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -516,8 +516,13 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int
ci.author_tz.buf,
show_raw_time));
}
- printf(" %*d) ",
- max_digits, ent->lno + 1 + cnt);
+ if (ent->ignored) {
+ printf(" %*d%c) ", max_digits - 1,
+ ent->lno + 1 + cnt, '*');
+ } else {
+ printf(" %*d) ", max_digits,
+ ent->lno + 1 + cnt);
+ }
}
if (reset)
fputs(reset, stdout);
@@ -603,6 +608,7 @@ static void find_alignment(struct blame_scoreboard *sb, int *option)
{
int longest_src_lines = 0;
int longest_dst_lines = 0;
+ int has_ignore = 0;
unsigned largest_score = 0;
struct blame_entry *e;
int compute_auto_abbrev = (abbrev < 0);
@@ -639,9 +645,11 @@ static void find_alignment(struct blame_scoreboard *sb, int *option)
longest_dst_lines = num;
if (largest_score < blame_entry_score(sb, e))
largest_score = blame_entry_score(sb, e);
+ if (e->ignored)
+ has_ignore = 1;
}
max_orig_digits = decimal_width(longest_src_lines);
- max_digits = decimal_width(longest_dst_lines);
+ max_digits = decimal_width(longest_dst_lines) + has_ignore;
max_score_digits = decimal_width(largest_score);
if (compute_auto_abbrev)
@@ -774,6 +782,43 @@ static int is_a_rev(const char *name)
return OBJ_NONE < oid_object_info(the_repository, &oid, NULL);
}
+static void handle_ignore_list(struct blame_scoreboard *sb,
+ struct string_list *ignores)
+{
+ struct string_list_item *i;
+ struct object_id oid;
+
+ oidset_init(&sb->ignores, 0);
+ for_each_string_list_item(i, ignores) {
+ if (get_oid_committish(i->string, &oid))
+ die(_("Can't find revision '%s' to ignore"), i->string);
+ oidset_insert(&sb->ignores, &oid);
+ }
+}
+
+static int handle_ignore_file(const char *path, struct string_list *ignores)
+{
+ FILE *fp = fopen(path, "r");
+ struct strbuf sb = STRBUF_INIT;
+
+ if (!fp)
+ return -1;
+ while (!strbuf_getline(&sb, fp)) {
+ const char *hash;
+
+ hash = strchr(sb.buf, '#');
+ if (hash)
+ strbuf_setlen(&sb, hash - sb.buf);
+ strbuf_trim(&sb);
+ if (!sb.len)
+ continue;
+ string_list_append(ignores, sb.buf);
+ }
+ fclose(fp);
+ strbuf_release(&sb);
+ return 0;
+}
+
int cmd_blame(int argc, const char **argv, const char *prefix)
{
struct rev_info revs;
@@ -785,8 +830,11 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
struct progress_info pi = { NULL, 0 };
struct string_list range_list = STRING_LIST_INIT_NODUP;
+ struct string_list ignore_list = STRING_LIST_INIT_DUP;
int output_option = 0, opt = 0;
int show_stats = 0;
+ int no_default_ignores = 0;
+ const char *ignore_file = NULL;
const char *revs_file = NULL;
const char *contents_from = NULL;
const struct option options[] = {
@@ -806,6 +854,9 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
OPT_BIT('s', NULL, &output_option, N_("Suppress author name and timestamp (Default: off)"), OUTPUT_NO_AUTHOR),
OPT_BIT('e', "show-email", &output_option, N_("Show author email instead of name (Default: off)"), OUTPUT_SHOW_EMAIL),
OPT_BIT('w', NULL, &xdl_opts, N_("Ignore whitespace differences"), XDF_IGNORE_WHITESPACE),
+ OPT_STRING_LIST('i', NULL, &ignore_list, N_("rev"), N_("Ignore <rev> when blaming")),
+ OPT_BOOL(0, "no-default-ignores", &no_default_ignores, N_("Do not ignore revisions from the .git-blame-ignore-revs file")),
+ OPT_STRING(0, "ignore-file", &ignore_file, N_("file"), N_("Ignore revisions from <file>")),
OPT_BIT(0, "color-lines", &output_option, N_("color redundant metadata from previous line differently"), OUTPUT_COLOR_LINE),
OPT_BIT(0, "color-by-age", &output_option, N_("color lines by age"), OUTPUT_SHOW_AGE_WITH_COLOR),
@@ -987,6 +1038,13 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
argv[argc - 1] = "--";
}
+ if (!no_default_ignores)
+ handle_ignore_file(".git-blame-ignore-revs", &ignore_list);
+ if (ignore_file) {
+ if (handle_ignore_file(ignore_file, &ignore_list))
+ die(_("Unable to open ignore-file '%s'"), ignore_file);
+ }
+
revs.disable_stdin = 1;
setup_revisions(argc, argv, &revs, NULL);
@@ -995,6 +1053,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
sb.contents_from = contents_from;
sb.reverse = reverse;
sb.repo = the_repository;
+ handle_ignore_list(&sb, &ignore_list);
+ string_list_clear(&ignore_list, 0);
setup_scoreboard(&sb, path, &o);
lno = sb.num_lines;
--
2.20.1.97.g81188d93c3-goog
^ permalink raw reply related
* Re: [RFC PATCH 1/1] filter-options: Expand abbreviated numbers
From: Josh Steadmon @ 2019-01-07 21:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqsgyaircj.fsf@gitster-ct.c.googlers.com>
On 2019.01.02 15:15, Junio C Hamano wrote:
> Josh Steadmon <steadmon@google.com> writes:
>
> > When communicating with a remote server or a subprocess, use expanded
> > numbers rather than abbreviated numbers in the object filter spec (e.g.
> > "limit:blob=1k" becomes "limit:blob=1024").
> >
> > Update the protocol docs to note that clients should always perform this
> > expansion, to allow for more compatibility between server
> > implementations.
> >
> > Signed-off-by: Josh Steadmon <steadmon@google.com>
> > ---
> > Documentation/technical/protocol-v2.txt | 5 ++++-
> > builtin/clone.c | 6 +++++-
> > builtin/fetch.c | 7 ++++++-
> > fetch-pack.c | 15 ++++++++++++---
> > list-objects-filter-options.c | 20 ++++++++++++++++++--
> > list-objects-filter-options.h | 17 +++++++++++++++--
> > t/t6112-rev-list-filters-objects.sh | 17 +++++++++++++++++
> > transport-helper.c | 13 +++++++++----
> > upload-pack.c | 7 +++++--
> > 9 files changed, 91 insertions(+), 16 deletions(-)
> >
> > diff --git a/Documentation/technical/protocol-v2.txt b/Documentation/technical/protocol-v2.txt
> > index 09e4e0273f..d001372404 100644
> > --- a/Documentation/technical/protocol-v2.txt
> > +++ b/Documentation/technical/protocol-v2.txt
> > @@ -296,7 +296,10 @@ included in the client's request:
> > Request that various objects from the packfile be omitted
> > using one of several filtering techniques. These are intended
> > for use with partial clone and partial fetch operations. See
> > - `rev-list` for possible "filter-spec" values.
> > + `rev-list` for possible "filter-spec" values. Clients MUST
> > + translate abbreviated numbers (e.g. "1k") into fully-expanded
> > + numbers (e.g. "1024") on the client side, so that the server
> > + does not need to implement unit parsing.
>
> I suspect that it is too late now to retroactively say "MUST" here.
> The best we may be able to do is to say "The sender SHOULD send a
> plain integer without unit and the receiver SHOULD be prepared to
> scale an integer with unit".
>
In that case, do you think we should also specify that units should be
interpreted as powers-of-2 rather than powers-of-10?
^ permalink raw reply
* Re: [PATCH] config.mak.dev: add -Wformat
From: Jonathan Nieder @ 2019-01-07 21:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: Thomas Gummerer, Jeff King, git, Josh Steadmon, Masaya Suzuki
In-Reply-To: <xmqqtvik9z69.fsf@gitster-ct.c.googlers.com>
Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
>>> From: Thomas Gummerer <t.gummerer@gmail.com>
>>> Date: Fri, 12 Oct 2018 19:40:37 +0100
>>> Subject: [PATCH] config.mak.dev: add -Wformat
>
> Thanks. I noticed, before merging the topic to 'next', that I
> needed to retitle this further. I'd use something like this.
>
> Subject: config.mak.dev: add -Wall to help autoconf users
With that change, it would still be
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
:)
Thanks.
^ permalink raw reply
* Re: Feature request: --preserve-merges to add "exec git merge ..." instead of "pick ..."
From: Andreas Hennings @ 2019-01-07 21:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqq36q4berk.fsf@gitster-ct.c.googlers.com>
It sounds good!
I was using git version 2.7.4 all the time. I should have checked
before posting here :)
Now trying 2.20.1
From "git help rebase":
By default, or when no-rebase-cousins was specified,
commits which do not have <upstream> as direct ancestor will keep
their
original branch point, i.e. commits that would be excluded
by gitlink:git-log[1]'s --ancestry-path option will keep their
original ancestry by default. If the rebase-cousins mode is
turned on, such commits are instead rebased onto <upstream> (or
<onto>, if specified).
I am not sure if this criterion (which ancestors it has) will always
produce the behavior I am looking for.
I will have to experiment a bit.
Thanks for now, I will post again with new thoughts once I have done
some experiments.
On Mon, 7 Jan 2019 at 17:42, Junio C Hamano <gitster@pobox.com> wrote:
>
> Andreas Hennings <andreas@dqxtech.net> writes:
>
> > This means we need a rebase operation where:
> > - The commits of the acceptance branch itself are being replaced.
> > - The commits of the feature branches remain as they are.
> >
> > A "git rebase --preserve-merges" almost does this, but not really.
>
> This wished-for feature sounds to me more like the "first-parent"
> mode that has been discussed a few times in the past but never
> materialized.
>
> The preserve-merges mode is getting abandoned by the original author
> as unsalvageable. Have you tried the rebase-merges mode? That may
> let you choose replaying only the merge commits on the acceptance
> branch without touching the tips of the feature branches getting
> merged.
^ permalink raw reply
* [PATCH] git-p4: fix problem when p4 login is not necessary
From: Peter Osterlund @ 2019-01-07 20:51 UTC (permalink / raw)
To: Git Users; +Cc: gitster, Luke Diamand
In a perforce setup where login is not required, communication fails
because p4_check_access does not understand the response from the p4
client. Fixed by detecting and ignoring the "info" response.
Signed-off-by: Peter Osterlund <peterosterlund2@gmail.com>
---
git-p4.py | 2 ++
1 file changed, 2 insertions(+)
diff --git a/git-p4.py b/git-p4.py
index 1998c3e141..3e12774f96 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -332,6 +332,8 @@ def p4_check_access(min_expiration=1):
die_bad_access("p4 error: {0}".format(data))
else:
die_bad_access("unknown error")
+ elif code == "info":
+ return
else:
die_bad_access("unknown error code {0}".format(code))
--
Peter Osterlund - peterosterlund2@gmail.com
http://hem.bredband.net/petero2b
^ 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