* Re: [PATCH] doc: fix merge-base ASCII art tab spacing
From: Junio C Hamano @ 2016-10-21 22:54 UTC (permalink / raw)
To: Philip Oakley; +Cc: GitList, Jeff King, Johannes Schindelin
In-Reply-To: <D861234B3E78496DBA70EE63B2BCDB96@PhilipOakley>
"Philip Oakley" <philipoakley@iee.org> writes:
> It appears that acciidoctor sees the art as being a separated
> mono-spaced block, with border/background as locally
> appropriate. While the asciidoc looks to simply change to mono-spaced
> text.
FWIW, I changed my mind and your patch is now queued on 'next'.
Thanks.
^ permalink raw reply
* Re: generating combined diff without an existing merge commit
From: Junio C Hamano @ 2016-10-21 22:41 UTC (permalink / raw)
To: Jacob Keller; +Cc: Git mailing list
In-Reply-To: <CA+P7+xoTO6MqdBjekbSpSs=w_dJ-1h_ypMDAo50mm95iTOo9Pw@mail.gmail.com>
Jacob Keller <jacob.keller@gmail.com> writes:
> Turns out that somehow I must have messed up my command because "git
> diff <treeish> <treeish> <treeish>" does indeed do exactly what I
> want.
Thanks. I was wondering why it didn't work for you, as the feature
was done exactly to support that use case ;-)
^ permalink raw reply
* [PATCH 3/3] transport: allow summary-width to be computed dynamically
From: Junio C Hamano @ 2016-10-21 22:39 UTC (permalink / raw)
To: git
In-Reply-To: <20161021223927.26364-1-gitster@pobox.com>
Now we have identified three callchains that have a set of refs that
they want to show their <old, new> object names in an aligned output,
we can replace their reference to the constant TRANSPORT_SUMMARY_WIDTH
with a helper function call to transport_summary_width() that takes
the set of ref as a parameter. This step does not yet iterate over
the refs and compute, which is left as an exercise to the readers.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/fetch.c | 4 ++--
transport.c | 7 ++++++-
transport.h | 2 +-
3 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 40696e5338..09813cd826 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -722,7 +722,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
char *url;
const char *filename = dry_run ? "/dev/null" : git_path_fetch_head();
int want_status;
- int summary_width = TRANSPORT_SUMMARY_WIDTH;
+ int summary_width = transport_summary_width(ref_map);
fp = fopen(filename, "a");
if (!fp)
@@ -906,7 +906,7 @@ static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map,
int url_len, i, result = 0;
struct ref *ref, *stale_refs = get_stale_heads(refs, ref_count, ref_map);
char *url;
- int summary_width = TRANSPORT_SUMMARY_WIDTH;
+ int summary_width = transport_summary_width(stale_refs);
const char *dangling_msg = dry_run
? _(" (%s will become dangling)")
: _(" (%s has become dangling)");
diff --git a/transport.c b/transport.c
index ec02b78924..d4b8bf5f25 100644
--- a/transport.c
+++ b/transport.c
@@ -429,6 +429,11 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count,
return 1;
}
+int transport_summary_width(const struct ref *refs)
+{
+ return (2 * FALLBACK_DEFAULT_ABBREV + 3);
+}
+
void transport_print_push_status(const char *dest, struct ref *refs,
int verbose, int porcelain, unsigned int *reject_reasons)
{
@@ -436,7 +441,7 @@ void transport_print_push_status(const char *dest, struct ref *refs,
int n = 0;
unsigned char head_sha1[20];
char *head;
- int summary_width = TRANSPORT_SUMMARY_WIDTH;
+ int summary_width = transport_summary_width(refs);
head = resolve_refdup("HEAD", RESOLVE_REF_READING, head_sha1, NULL);
diff --git a/transport.h b/transport.h
index e783377e40..706d99e818 100644
--- a/transport.h
+++ b/transport.h
@@ -142,7 +142,7 @@ struct transport {
#define TRANSPORT_PUSH_ATOMIC 8192
#define TRANSPORT_PUSH_OPTIONS 16384
-#define TRANSPORT_SUMMARY_WIDTH (2 * FALLBACK_DEFAULT_ABBREV + 3)
+extern int transport_summary_width(const struct ref *refs);
/* Returns a transport suitable for the url */
struct transport *transport_get(struct remote *, const char *);
--
2.10.1-723-g2384e83bc3
^ permalink raw reply related
* [PATCH 1/3] transport: pass summary_width down the callchain
From: Junio C Hamano @ 2016-10-21 22:39 UTC (permalink / raw)
To: git
In-Reply-To: <20161021223927.26364-1-gitster@pobox.com>
The callchain that originates at transport_print_push_status()
eventually hits a single leaf function, print_ref_status(), that is
used to show from what old object to what new object a ref got
updated, and the width of the part that shows old and new object
names used a constant TRANSPORT_SUMMARY_WIDTH.
Teach the callchain to pass the width down from the top instead.
This allows a future enhancement to compute the necessary display
width before calling down this callchain.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
transport.c | 63 +++++++++++++++++++++++++++++++++++++------------------------
1 file changed, 38 insertions(+), 25 deletions(-)
diff --git a/transport.c b/transport.c
index 94d6dc3725..ec02b78924 100644
--- a/transport.c
+++ b/transport.c
@@ -295,7 +295,9 @@ void transport_update_tracking_ref(struct remote *remote, struct ref *ref, int v
}
}
-static void print_ref_status(char flag, const char *summary, struct ref *to, struct ref *from, const char *msg, int porcelain)
+static void print_ref_status(char flag, const char *summary,
+ struct ref *to, struct ref *from, const char *msg,
+ int porcelain, int summary_width)
{
if (porcelain) {
if (from)
@@ -307,7 +309,7 @@ static void print_ref_status(char flag, const char *summary, struct ref *to, str
else
fprintf(stdout, "%s\n", summary);
} else {
- fprintf(stderr, " %c %-*s ", flag, TRANSPORT_SUMMARY_WIDTH, summary);
+ fprintf(stderr, " %c %-*s ", flag, summary_width, summary);
if (from)
fprintf(stderr, "%s -> %s", prettify_refname(from->name), prettify_refname(to->name));
else
@@ -321,15 +323,16 @@ static void print_ref_status(char flag, const char *summary, struct ref *to, str
}
}
-static void print_ok_ref_status(struct ref *ref, int porcelain)
+static void print_ok_ref_status(struct ref *ref, int porcelain, int summary_width)
{
if (ref->deletion)
- print_ref_status('-', "[deleted]", ref, NULL, NULL, porcelain);
+ print_ref_status('-', "[deleted]", ref, NULL, NULL,
+ porcelain, summary_width);
else if (is_null_oid(&ref->old_oid))
print_ref_status('*',
(starts_with(ref->name, "refs/tags/") ? "[new tag]" :
"[new branch]"),
- ref, ref->peer_ref, NULL, porcelain);
+ ref, ref->peer_ref, NULL, porcelain, summary_width);
else {
struct strbuf quickref = STRBUF_INIT;
char type;
@@ -349,12 +352,14 @@ static void print_ok_ref_status(struct ref *ref, int porcelain)
strbuf_add_unique_abbrev(&quickref, ref->new_oid.hash,
DEFAULT_ABBREV);
- print_ref_status(type, quickref.buf, ref, ref->peer_ref, msg, porcelain);
+ print_ref_status(type, quickref.buf, ref, ref->peer_ref, msg,
+ porcelain, summary_width);
strbuf_release(&quickref);
}
}
-static int print_one_push_status(struct ref *ref, const char *dest, int count, int porcelain)
+static int print_one_push_status(struct ref *ref, const char *dest, int count,
+ int porcelain, int summary_width)
{
if (!count) {
char *url = transport_anonymize_url(dest);
@@ -364,56 +369,60 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
switch(ref->status) {
case REF_STATUS_NONE:
- print_ref_status('X', "[no match]", ref, NULL, NULL, porcelain);
+ print_ref_status('X', "[no match]", ref, NULL, NULL,
+ porcelain, summary_width);
break;
case REF_STATUS_REJECT_NODELETE:
print_ref_status('!', "[rejected]", ref, NULL,
- "remote does not support deleting refs", porcelain);
+ "remote does not support deleting refs",
+ porcelain, summary_width);
break;
case REF_STATUS_UPTODATE:
print_ref_status('=', "[up to date]", ref,
- ref->peer_ref, NULL, porcelain);
+ ref->peer_ref, NULL, porcelain, summary_width);
break;
case REF_STATUS_REJECT_NONFASTFORWARD:
print_ref_status('!', "[rejected]", ref, ref->peer_ref,
- "non-fast-forward", porcelain);
+ "non-fast-forward", porcelain, summary_width);
break;
case REF_STATUS_REJECT_ALREADY_EXISTS:
print_ref_status('!', "[rejected]", ref, ref->peer_ref,
- "already exists", porcelain);
+ "already exists", porcelain, summary_width);
break;
case REF_STATUS_REJECT_FETCH_FIRST:
print_ref_status('!', "[rejected]", ref, ref->peer_ref,
- "fetch first", porcelain);
+ "fetch first", porcelain, summary_width);
break;
case REF_STATUS_REJECT_NEEDS_FORCE:
print_ref_status('!', "[rejected]", ref, ref->peer_ref,
- "needs force", porcelain);
+ "needs force", porcelain, summary_width);
break;
case REF_STATUS_REJECT_STALE:
print_ref_status('!', "[rejected]", ref, ref->peer_ref,
- "stale info", porcelain);
+ "stale info", porcelain, summary_width);
break;
case REF_STATUS_REJECT_SHALLOW:
print_ref_status('!', "[rejected]", ref, ref->peer_ref,
- "new shallow roots not allowed", porcelain);
+ "new shallow roots not allowed",
+ porcelain, summary_width);
break;
case REF_STATUS_REMOTE_REJECT:
print_ref_status('!', "[remote rejected]", ref,
- ref->deletion ? NULL : ref->peer_ref,
- ref->remote_status, porcelain);
+ ref->deletion ? NULL : ref->peer_ref,
+ ref->remote_status, porcelain, summary_width);
break;
case REF_STATUS_EXPECTING_REPORT:
print_ref_status('!', "[remote failure]", ref,
- ref->deletion ? NULL : ref->peer_ref,
- "remote failed to report status", porcelain);
+ ref->deletion ? NULL : ref->peer_ref,
+ "remote failed to report status",
+ porcelain, summary_width);
break;
case REF_STATUS_ATOMIC_PUSH_FAILED:
print_ref_status('!', "[rejected]", ref, ref->peer_ref,
- "atomic push failed", porcelain);
+ "atomic push failed", porcelain, summary_width);
break;
case REF_STATUS_OK:
- print_ok_ref_status(ref, porcelain);
+ print_ok_ref_status(ref, porcelain, summary_width);
break;
}
@@ -427,25 +436,29 @@ void transport_print_push_status(const char *dest, struct ref *refs,
int n = 0;
unsigned char head_sha1[20];
char *head;
+ int summary_width = TRANSPORT_SUMMARY_WIDTH;
head = resolve_refdup("HEAD", RESOLVE_REF_READING, head_sha1, NULL);
if (verbose) {
for (ref = refs; ref; ref = ref->next)
if (ref->status == REF_STATUS_UPTODATE)
- n += print_one_push_status(ref, dest, n, porcelain);
+ n += print_one_push_status(ref, dest, n,
+ porcelain, summary_width);
}
for (ref = refs; ref; ref = ref->next)
if (ref->status == REF_STATUS_OK)
- n += print_one_push_status(ref, dest, n, porcelain);
+ n += print_one_push_status(ref, dest, n,
+ porcelain, summary_width);
*reject_reasons = 0;
for (ref = refs; ref; ref = ref->next) {
if (ref->status != REF_STATUS_NONE &&
ref->status != REF_STATUS_UPTODATE &&
ref->status != REF_STATUS_OK)
- n += print_one_push_status(ref, dest, n, porcelain);
+ n += print_one_push_status(ref, dest, n,
+ porcelain, summary_width);
if (ref->status == REF_STATUS_REJECT_NONFASTFORWARD) {
if (head != NULL && !strcmp(head, ref->name))
*reject_reasons |= REJECT_NON_FF_HEAD;
--
2.10.1-723-g2384e83bc3
^ permalink raw reply related
* [PATCH 0/3] fetch output is ugly in 'next'
From: Junio C Hamano @ 2016-10-21 22:39 UTC (permalink / raw)
To: git
In-Reply-To: <xmqqa8dxbb9r.fsf@gitster.mtv.corp.google.com>
It turns out that there are three codepaths, all of which have a set
of refs that they want to show them using TRANSPORT_SUMMARY_WIDTH
constant. The two preparatory steps turn the constant used at the
leaf level into a parameter that is passed down through these
callchains, and the last step introduces a helper function that can
be used to compute the appropriate width to be fed to these
callchains.
Junio C Hamano (3):
transport: pass summary_width down the callchain
fetch: pass summary_width down the callchain
transport: allow summary-width to be computed dynamically
builtin/fetch.c | 37 +++++++++++++++++--------------
transport.c | 68 ++++++++++++++++++++++++++++++++++++---------------------
transport.h | 2 +-
3 files changed, 65 insertions(+), 42 deletions(-)
--
2.10.1-723-g2384e83bc3
^ permalink raw reply
* [PATCH 2/3] fetch: pass summary_width down the callchain
From: Junio C Hamano @ 2016-10-21 22:39 UTC (permalink / raw)
To: git
In-Reply-To: <20161021223927.26364-1-gitster@pobox.com>
The leaf function on the "fetch" side that uses TRANSPORT_SUMMARY_WIDTH
constant is builtin/fetch.c::format_display() and it has two distinct
callchains. The one that reports the primary result of fetch originates
at store_updated_refs(); the other one that reports the pruning of
the remote-tracking refs originates at prune_refs().
Teach these two places to pass summary_width down the callchain,
just like we did for the "push" side in the previous commit.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/fetch.c | 37 +++++++++++++++++++++----------------
1 file changed, 21 insertions(+), 16 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index a9f12cc5cf..40696e5338 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -17,9 +17,6 @@
#include "argv-array.h"
#include "utf8.h"
-#define TRANSPORT_SUMMARY(x) \
- (int)(TRANSPORT_SUMMARY_WIDTH + strlen(x) - gettext_width(x)), (x)
-
static const char * const builtin_fetch_usage[] = {
N_("git fetch [<options>] [<repository> [<refspec>...]]"),
N_("git fetch [<options>] <group>"),
@@ -569,9 +566,12 @@ static void print_compact(struct strbuf *display,
static void format_display(struct strbuf *display, char code,
const char *summary, const char *error,
- const char *remote, const char *local)
+ const char *remote, const char *local,
+ int summary_width)
{
- strbuf_addf(display, "%c %-*s ", code, TRANSPORT_SUMMARY(summary));
+ int width = (summary_width + strlen(summary) - gettext_width(summary));
+
+ strbuf_addf(display, "%c %-*s ", code, width, summary);
if (!compact_format)
print_remote_to_local(display, remote, local);
else
@@ -583,7 +583,8 @@ static void format_display(struct strbuf *display, char code,
static int update_local_ref(struct ref *ref,
const char *remote,
const struct ref *remote_ref,
- struct strbuf *display)
+ struct strbuf *display,
+ int summary_width)
{
struct commit *current = NULL, *updated;
enum object_type type;
@@ -597,7 +598,7 @@ static int update_local_ref(struct ref *ref,
if (!oidcmp(&ref->old_oid, &ref->new_oid)) {
if (verbosity > 0)
format_display(display, '=', _("[up to date]"), NULL,
- remote, pretty_ref);
+ remote, pretty_ref, summary_width);
return 0;
}
@@ -611,7 +612,7 @@ static int update_local_ref(struct ref *ref,
*/
format_display(display, '!', _("[rejected]"),
_("can't fetch in current branch"),
- remote, pretty_ref);
+ remote, pretty_ref, summary_width);
return 1;
}
@@ -621,7 +622,7 @@ static int update_local_ref(struct ref *ref,
r = s_update_ref("updating tag", ref, 0);
format_display(display, r ? '!' : 't', _("[tag update]"),
r ? _("unable to update local ref") : NULL,
- remote, pretty_ref);
+ remote, pretty_ref, summary_width);
return r;
}
@@ -654,7 +655,7 @@ static int update_local_ref(struct ref *ref,
r = s_update_ref(msg, ref, 0);
format_display(display, r ? '!' : '*', what,
r ? _("unable to update local ref") : NULL,
- remote, pretty_ref);
+ remote, pretty_ref, summary_width);
return r;
}
@@ -670,7 +671,7 @@ static int update_local_ref(struct ref *ref,
r = s_update_ref("fast-forward", ref, 1);
format_display(display, r ? '!' : ' ', quickref.buf,
r ? _("unable to update local ref") : NULL,
- remote, pretty_ref);
+ remote, pretty_ref, summary_width);
strbuf_release(&quickref);
return r;
} else if (force || ref->force) {
@@ -685,12 +686,12 @@ static int update_local_ref(struct ref *ref,
r = s_update_ref("forced-update", ref, 1);
format_display(display, r ? '!' : '+', quickref.buf,
r ? _("unable to update local ref") : _("forced update"),
- remote, pretty_ref);
+ remote, pretty_ref, summary_width);
strbuf_release(&quickref);
return r;
} else {
format_display(display, '!', _("[rejected]"), _("non-fast-forward"),
- remote, pretty_ref);
+ remote, pretty_ref, summary_width);
return 1;
}
}
@@ -721,6 +722,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
char *url;
const char *filename = dry_run ? "/dev/null" : git_path_fetch_head();
int want_status;
+ int summary_width = TRANSPORT_SUMMARY_WIDTH;
fp = fopen(filename, "a");
if (!fp)
@@ -830,13 +832,14 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
strbuf_reset(¬e);
if (ref) {
- rc |= update_local_ref(ref, what, rm, ¬e);
+ rc |= update_local_ref(ref, what, rm, ¬e,
+ summary_width);
free(ref);
} else
format_display(¬e, '*',
*kind ? kind : "branch", NULL,
*what ? what : "HEAD",
- "FETCH_HEAD");
+ "FETCH_HEAD", summary_width);
if (note.len) {
if (verbosity >= 0 && !shown_url) {
fprintf(stderr, _("From %.*s\n"),
@@ -903,6 +906,7 @@ static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map,
int url_len, i, result = 0;
struct ref *ref, *stale_refs = get_stale_heads(refs, ref_count, ref_map);
char *url;
+ int summary_width = TRANSPORT_SUMMARY_WIDTH;
const char *dangling_msg = dry_run
? _(" (%s will become dangling)")
: _(" (%s has become dangling)");
@@ -938,7 +942,8 @@ static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map,
shown_url = 1;
}
format_display(&sb, '-', _("[deleted]"), NULL,
- _("(none)"), prettify_refname(ref->name));
+ _("(none)"), prettify_refname(ref->name),
+ summary_width);
fprintf(stderr, " %s\n",sb.buf);
strbuf_release(&sb);
warn_dangling_symref(stderr, dangling_msg, ref->name);
--
2.10.1-723-g2384e83bc3
^ permalink raw reply related
* Re: [BUG] fetch output is ugly in 'next'
From: Junio C Hamano @ 2016-10-21 22:14 UTC (permalink / raw)
To: Jeff King; +Cc: Duy Nguyen, Git Mailing List
In-Reply-To: <20161021214218.u46qf3lch3wwiutp@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> The "right" fix is to queue up the list of ref updates to print, find
> the abbreviations for each, and then print them all in one shot, knowing
> ahead of time the size necessary to align them. This could also let us
> improve the name-alignment.
Heh, that was what I was starting to code while listening to
Jonathan and Stefan talk in a meeting ;-)
^ permalink raw reply
* Re: generating combined diff without an existing merge commit
From: Jacob Keller @ 2016-10-21 22:01 UTC (permalink / raw)
To: Git mailing list
In-Reply-To: <CA+P7+xrN-_zP40uAUGtqZW+OO4D4Z65SiPRykdKvauO1zgNNcQ@mail.gmail.com>
On Fri, Oct 21, 2016 at 2:40 PM, Jacob Keller <jacob.keller@gmail.com> wrote:
> Hi,
>
> I recently determined that I can produce an interdiff for a series
> that handles rebasing nicely and shows the conflicts resolved when
> rebasing plus any other changes.
>
> The basic idea is something like the following, assuming that v1 is a
> tag that points to the first version, v2 is a tag that points to the
> rebased new version, and base is a tag that points to the new base of
> the series (ie: the upstream if the v2 is on a branch and has been
> fully rebased)
>
> git checkout v1
> git merge base
> #perform any further edits to get everything looking like v2
> git commit
> git show -cc HEAD
>
> This is also equivalent to the following without having to actually do
> the merge manually:
>
> git commit-tree v2^{head} -p v1 -p master -m "some merge message"
> git show <output from the commit tree above)
>
> this nicely shows us the combined diff format which correctly shows
> any conflicts required to fix up during the rebase (which we already
> did because we have v2) and it also shows any *other* changes caused
> by v2 but without showing changes which we didn't actually make. (I
> think?)
>
> The result is that we can nicely see what was required to produce v2
> from v1 but without being cluttered by what changed in base.
>
> However, I have to actually generate the commit to do this. I am
> wondering if it is possible today to actually just do something like:
>
> git diff <treeish> <treeish> <treeish> and get the result that I want?
>
> I've already started digging to see if I can do that but haven't found
> anything yet.
>
> Thanks,
> Jake
Turns out that somehow I must have messed up my command because "git
diff <treeish> <treeish> <treeish>" does indeed do exactly what I
want.
Thanks,
Jake
^ permalink raw reply
* Re: [PATCH 2/3] test-lib: add --verbose-log option
From: Jeff King @ 2016-10-21 21:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stefan Beller, Lars Schneider, git
In-Reply-To: <xmqq7f91d3tb.fsf@gitster.mtv.corp.google.com>
On Fri, Oct 21, 2016 at 10:12:16AM -0700, Junio C Hamano wrote:
> > -if test "$verbose" = "t"
> > +if test "$verbose_log" = "t"
> > +then
> > + exec 3>>"$GIT_TEST_TEE_OUTPUT_FILE" 4>&3
> > +elif test "$verbose" = "t"
> > then
> > exec 4>&2 3>&1
> > else
>
> OK, unlike "verbose" case where we give 3 (saved stdout) to 1 and 4
> (saved stderr) to 2, we send 3 to the output file and 4 to the same.
Your comment made me double-check whether we ought to be separating the
two in any way. Because my statement earlier in the thread that the
--verbose output goes to stdout is not _entirely_ true. It goes to
stdout and stderr, as written by the various programs in the tests.
So:
./t0000-whatever.sh -v | less
isn't quite sufficient. You really do want "2>&1" in there to catch
everything.
But for the case of "--tee", we already do so, in order to make sure it
all goes to the logfile. And since this option always implies "--tee",
it is correct here to send "3" and "4" to the same place.
So the patch is correct. But when I said earlier that people might be
annoyed if we just sent --verbose output to stderr, that probably isn't
true. We could perhaps make:
prove t0000-whatever.sh :: -v
"just work" by sending all of the verbose output to stderr. I don't
think it really matters, though. Nobody is doing that, because it's
pointless without "--tee".
-Peff
^ permalink raw reply
* Re: [BUG] fetch output is ugly in 'next'
From: Jeff King @ 2016-10-21 21:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Duy Nguyen, Git Mailing List
In-Reply-To: <xmqqd1itd4t2.fsf@gitster.mtv.corp.google.com>
On Fri, Oct 21, 2016 at 09:50:49AM -0700, Junio C Hamano wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
> > On Fri, Oct 21, 2016 at 7:11 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> >> Yeah.. replacing the 4 DEFAULT_ABBREV in fetch.c with something
> >> sensible should do it.
> >
> > Correction (if somebody will pick this up), it's
> > TRANSPORT_SUMMARY_WIDTH that needs to be adjusted, not those four.
>
> Yes, it used to be and it still is (2 * DEFAULT_ABBREV + 3) but in
> the new world order where default-abbrev is often -1 the expression
> does not make much sense.
>
> Perhaps something along this line?
>
> transport.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/transport.h b/transport.h
> index 18d2cf8275..5339fabbad 100644
> --- a/transport.h
> +++ b/transport.h
> @@ -127,7 +127,7 @@ struct transport {
> #define TRANSPORT_PUSH_CERT 2048
> #define TRANSPORT_PUSH_ATOMIC 4096
>
> -#define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)
> +#define TRANSPORT_SUMMARY_WIDTH (2 * (DEFAULT_ABBREV < 0 ? 7 : DEFAULT_ABBREV) + 3)
> #define TRANSPORT_SUMMARY(x) (int)(TRANSPORT_SUMMARY_WIDTH + strlen(x) - gettext_width(x)), (x)
That doesn't apply on 'next', because we have already done the
equivalent there. :)
The right way to spell "7" is FALLBACK_DEFAULT_ABBREV, which is handled
by your 65acfeacaa.
The remaining issue is that the static abbreviation is not nearly long
enough for git.git anymore; the auto-abbrev feature bumps my repo to a
minimum of 10 characters (it may only be 9 on a fresh clone; I have a
couple remotes and active work in progress). So this isn't exactly a
regression; it has always been the case that we may mis-align when the
abbreviations ended up longer than the minimum. It's just that it didn't
happen all that often in most repos (but it probably did constantly in
linux.git).
The simplest band-aid fix would be to compute TRANSPORT_SUMMARY_WIDTH on
the fly, taking into account the minimum found by actually counting the
objects. That at least gets us back to where we were, with it mostly
working and occasionally ugly when there's an oddball collision (for
git.git anyway; it probably makes the kernel output much nicer).
The "right" fix is to queue up the list of ref updates to print, find
the abbreviations for each, and then print them all in one shot, knowing
ahead of time the size necessary to align them. This could also let us
improve the name-alignment.
-Peff
^ permalink raw reply
* generating combined diff without an existing merge commit
From: Jacob Keller @ 2016-10-21 21:40 UTC (permalink / raw)
To: Git mailing list
Hi,
I recently determined that I can produce an interdiff for a series
that handles rebasing nicely and shows the conflicts resolved when
rebasing plus any other changes.
The basic idea is something like the following, assuming that v1 is a
tag that points to the first version, v2 is a tag that points to the
rebased new version, and base is a tag that points to the new base of
the series (ie: the upstream if the v2 is on a branch and has been
fully rebased)
git checkout v1
git merge base
#perform any further edits to get everything looking like v2
git commit
git show -cc HEAD
This is also equivalent to the following without having to actually do
the merge manually:
git commit-tree v2^{head} -p v1 -p master -m "some merge message"
git show <output from the commit tree above)
this nicely shows us the combined diff format which correctly shows
any conflicts required to fix up during the rebase (which we already
did because we have v2) and it also shows any *other* changes caused
by v2 but without showing changes which we didn't actually make. (I
think?)
The result is that we can nicely see what was required to produce v2
from v1 but without being cluttered by what changed in base.
However, I have to actually generate the commit to do this. I am
wondering if it is possible today to actually just do something like:
git diff <treeish> <treeish> <treeish> and get the result that I want?
I've already started digging to see if I can do that but haven't found
anything yet.
Thanks,
Jake
^ permalink raw reply
* Re: [PATCH] doc: fix merge-base ASCII art tab spacing
From: Philip Oakley @ 2016-10-21 21:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: GitList, Jeff King, Johannes Schindelin
In-Reply-To: <xmqqa8dyeebj.fsf@gitster.mtv.corp.google.com>
From: "Junio C Hamano" <gitster@pobox.com>
> Philip Oakley <philipoakley@iee.org> writes:
>
>> The doc-tool stack does not always respect the 'tab = 8 spaces' rule,
>> particularly the git-scm doc pages
>> https://git-scm.com/docs/git-merge-base
>> and the Git generated html pages.
>
> Sorry, but I do not understand this change.
>
> https://git.github.io/htmldocs/git-merge-base.html is "Git generated
> HTML page" and I do not see any breakage around the drawings this
> patch touches, and the fp-path series does not touch these drawings,
> either.
>
I'd been looking the Git for Windows output, which has the same breakage,
rather than yours. Apologies for failing to check there.
> If a broken "doc-tool stack" breaks the formatting, I'd prefer to
> see that "doc-tool stack" fixed, instead of working around by
The doc-tool stack is question is asciidoctor. It looks like it is an
explicit decision that the the 8 space tab substitution is deprecated in
these case (see http://asciidoctor.org/docs/user-manual/#migrate-deprecated
#96.3)
It appears that acciidoctor sees the art as being a separated mono-spaced
block, with border/background as locally appropriate. While the asciidoc
looks to simply change to mono-spaced text.
> updating the source they work on. Otherwise, the broken "doc-tool
> stack" will keep producing broken output next time a source that
> respects "tab is to skip to the next multiple of 8" rule is fed to
> it, no?
By avoiding tabs *within the art* we would also be tolerant of those who may
not have a set their tab spacing to 8 when viewing the raw text.
It's particularly the criss-cross diagram that needs fixed one way or
another (for the doc/doctor differences).
The update of the asciidoctor version for git-scm, as reported by peff isn't
sufficient for this case.
also cc'ing dscho as this breakage shows in GfW (issue 923)
Philip
^ permalink raw reply
* Re: [PATCHv3] submodule--helper: normalize funny urls
From: Stefan Beller @ 2016-10-21 20:56 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Sixt, Johannes Schindelin, git@vger.kernel.org, Karl A.,
Dennis Kaarsemaker, Jonathan Nieder
In-Reply-To: <xmqqwph5m6th.fsf@gitster.mtv.corp.google.com>
On Tue, Oct 18, 2016 at 7:05 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>>> I am not sure. Certainly we would want to make sure that the normal
>>> case (i.e. no funny trailing junk) to work correctly, but we do want
>>> to protect the fix from future breakage as well, no?
>>
>> Exactly. So not intermediate "root" that we clone from, but adapting the
>> relative URLs. Maybe half the broken tests can switch to 'root' and the others
>> go with the current behavior of cloning . to super.
>>>
>>> Perhaps we can do a preliminary step to update tests to primarily
>>> check the cases that do not involve URL with trailing "/." by either
>>> doing that double clone, or more preferrably, clone from "$(pwd)"
>>> and adjust the incorrect submodule reference that have been relying
>>> on the buggy behaviour. With that "root" trick, the test would pass
>>> with or without the fix under discussion, right?
>>
>> I assume so, (not tested).
>
> OK. Thanks for sticking with it.
Ok, the root trick works fine without the fix, however we preferrably
want to fix it
without double cloning, then the fix becomes a bit harder to follow:
instead of
git clone . super
we do
git clone "$(pwd)" super &&
git -C super config --unset remote.origin.url &&
instead, such that the relative urls work the same way.
(The super becomes its own authoritative source of truth with no upstream.
In that case the url is relative to the superproject, e.g.
git -C super submodule add ../submodule
will then resolve the relative path from super/../submodule to be just
as before, where
we bugily used $(git config remote.origin.url =
...super/.)/../submodule, which due to the
but resolved to the submodule as well (as /. was counted as a directory).
^ permalink raw reply
* Re: Drastic jump in the time required for the test suite
From: René Scharfe @ 2016-10-21 20:28 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Johannes Schindelin, Junio C Hamano, Git Mailing List
In-Reply-To: <CACsJy8D-o8BeaV1jrL7ADXgVwJ=KwnBCHTCN_fuCGvS1VVpU6A@mail.gmail.com>
Am 21.10.2016 um 12:59 schrieb Duy Nguyen:
> On Thu, Oct 20, 2016 at 11:40 PM, René Scharfe <l.s.r@web.de> wrote:
>> I get this on WSL with prove -j8:
>>
>> Files=750, Tests=13657, 906 wallclock secs ( 8.51 usr 17.17 sys + 282.62 cusr 3731.85 csys = 4040.15 CPU)
>>
>> And this for a run on Debian inside a Hyper-V VM on the same system:
>>
>> Files=759, Tests=13895, 99 wallclock secs ( 4.81 usr 1.06 sys + 39.70 cusr 25.82 csys = 71.39 CPU)
>>
>> All tests pass on master.
>
> Thank you for doing this. 10 times slower is probably not worth
> following up (though absolute numbers still look amazing, you have
> some beefy machine there).
Thanks, but it's not too impressive: Xeon E3-1231 v3, Crucial BX100 SSD,
8GB RAM. I wonder how much faster a brand-new CPU with more RAM and a
PCIe SSD would be..
René
^ permalink raw reply
* Re: Drastic jump in the time required for the test suite
From: René Scharfe @ 2016-10-21 19:57 UTC (permalink / raw)
To: Matthieu Moy
Cc: Duy Nguyen, Johannes Schindelin, Junio C Hamano, Git Mailing List
In-Reply-To: <vpqd1itdf02.fsf@anie.imag.fr>
Am 21.10.2016 um 15:10 schrieb Matthieu Moy:
> René Scharfe <l.s.r@web.de> writes:
>
>> I get this on WSL with prove -j8:
>>
>> Files=750, Tests=13657, 906 wallclock secs ( 8.51 usr 17.17 sys + 282.62 cusr 3731.85 csys = 4040.15 CPU)
>>
>> And this for a run on Debian inside a Hyper-V VM on the same system:
>>
>> Files=759, Tests=13895, 99 wallclock secs ( 4.81 usr 1.06 sys + 39.70 cusr 25.82 csys = 71.39 CPU)
>
> What about the same without WSL on windows?
Files=755, Tests=14894, 1774 wallclock secs ( 9.31 usr 9.58 sys + 821.87 cusr 1961.23 csys = 2801.99 CPU)
René
^ permalink raw reply
* Re: RFC Failover url for fetches?
From: Junio C Hamano @ 2016-10-21 19:03 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org
In-Reply-To: <CAGZ79kaPdSfY_DXL6BDQ9pAma8p61r4m1n81VTxPHYi8zQuZfA@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> So when pushing it is possible to have multiple urls
> (remote.<name>.url) configured.
>
> When fetching only the first configured url is considered.
> Would it make sense to allow multiple urls and
> try them one by one until one works?
I do not think the two are related. Pushing to multiple is not "I
want to update at least one of them" in the first place.
As to fetching from two or more places as "fallback", I am
moderately negative to add it as a dumb feature that does nothing
more than "My fetch from A failed, so let's blindly try it from B".
I'd prefer to keep the "My fetch from A is failing" knowledge near
the surface of end user's consciousness as a mechanism to pressure A
to fix it--that way everybody who is fetching from A benefits.
After all, doing "git remote add B" once (you'd need to tell the URL
for B anyway to Git) and issuing "git fetch B" after seeing your
regular "git fetch" fails once in a blue moon is not all that
cumbersome, I would think.
Some people _may_ have objection based on A and B going out of sync,
especially B may fall behind even yourself and cause non-ff errors,
but I personally am not worried about that, because when somebody
configures B as a fallback for A, there is an expectation that B is
kept reasonably up to date. It would be a problem if some refs are
expected to be constantly rewound at A (e.g. 'pu' in my tree) and
configured to always force-fetch, though. A stale B would silently
set such a branch in your repository back without failing.
^ permalink raw reply
* Re: [PATCH 1/2] t9000-addresses: update expected results after fix
From: Ramsay Jones @ 2016-10-21 18:44 UTC (permalink / raw)
To: Junio C Hamano, Matthieu Moy; +Cc: git
In-Reply-To: <xmqqh985d4x7.fsf@gitster.mtv.corp.google.com>
On 21/10/16 17:48, Junio C Hamano wrote:
> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>
>> e3fdbcc8e1 (parse_mailboxes: accept extra text after <...> address,
>> 2016-10-13) improved our in-house address parser and made it closer to
>> Mail::Address. As a consequence, some tests comparing it to
>> Mail::Address now pass, but e3fdbcc8e1 forgot to update the test.
>>
>> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
>> ---
>
> Thanks.
Yep, thanks for looking into this Matthieu.
I applied these cleanly (to both next and pu) and tested
on Linux and cygwin.
Thanks again.
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH v5 00/27] Prepare the sequencer for the upcoming rebase -i patches
From: Junio C Hamano @ 2016-10-21 18:40 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Jakub Narębski, Johannes Sixt, Ramsay Jones
In-Reply-To: <cover.1477052405.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> Changes vs v4:
I still do not understand (note that I am not saying "I do not
accept"--acceptance or rejection happens after an understandable
explanation is given, and "do not understand" means no such
explanation has been given yet) your justification behind adding a
technical debt to reimplement the author-script parser and not
sharing it with "git am" in 13/27.
As I pointed out, I think 22/27 is out of place in this series.
But other than these two points, the changes since v4 look minimum,
and they all looked good to me.
Thanks.
^ permalink raw reply
* Re: [PATCH v5 22/27] sequencer: teach write_message() to append an optional LF
From: Junio C Hamano @ 2016-10-21 18:32 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Jakub Narębski, Johannes Sixt, Ramsay Jones
In-Reply-To: <d858bd99a3ad3dea807f38b8bd42a221a9f7924c.1477052405.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> This commit prepares for future callers that will have a pointer/length
> to some text to be written that lacks an LF, yet an LF is desired.
> Instead of requiring the caller to append an LF to the buffer (and
> potentially allocate memory to do so), the write_message() function
> learns to append an LF at the end of the file.
As no existing callers need this, it probably is better left out and
added to the series that actually needs the new feature as a
preparatory step.
^ permalink raw reply
* Re: [PATCH v5 19/27] sequencer: stop releasing the strbuf in write_message()
From: Junio C Hamano @ 2016-10-21 18:30 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Jakub Narębski, Johannes Sixt, Ramsay Jones
In-Reply-To: <c89508366501c17e7363a7d5c631fce9b3750cd2.1477052405.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> Nothing in the name "write_message()" suggests that the function
> releases the strbuf passed to it. So let's release the strbuf in the
> caller instead.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
I agree that it makes quite a lot of sense from the point of view of
"taste in the API design".
> sequencer.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index d74fdce..745c86f 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -243,7 +243,6 @@ static int write_message(struct strbuf *msgbuf, const char *filename)
> return error_errno(_("Could not lock '%s'"), filename);
> if (write_in_full(msg_fd, msgbuf->buf, msgbuf->len) < 0)
> return error_errno(_("Could not write to %s"), filename);
> - strbuf_release(msgbuf);
> if (commit_lock_file(&msg_file) < 0)
> return error(_("Error wrapping up %s."), filename);
>
> @@ -759,6 +758,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
> free_commit_list(common);
> free_commit_list(remotes);
> }
> + strbuf_release(&msgbuf);
>
> /*
> * If the merge was clean or if it failed due to conflict, we write
^ permalink raw reply
* Re: [PATCH] daemon, path.c: fix a bug with ~ in repo paths
From: Junio C Hamano @ 2016-10-21 18:23 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Luke Shumaker, Git Mailing List, Jeff King
In-Reply-To: <xmqqlgxhd77u.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> The amount of changes is unbelievable for fixing such a rare case
>> though. I wonder if we can just detect this in daemon.c and pass
>> "./~foo/bar" instead of "~foo/bar" to enter_repo() in non-strict mode
>> to "disable" expand_user_path(). If it works, it's much simpler
>> changes (even though a bit hacky)
>
> Conceptually, it ought to be updating the code that says "Does it
> begin with a tilde? Then try to do user-path expansion and fail if
> that is not enabled and if it succeeds use the resulting directory"
> to "Is user-path enabled and does it begin with a tilde? Then try
> to do user-path expansion and if it succeeds use the resulting
> directory". Compared to that mental model we have with this
> codepath, I agree that the change does look quite invasive and
> large.
>
> It is OK for a change to be large, as long as the end result is
> easier to read and understand than the original. I am undecided if
> that is the case with this patch, though.
I am still not convinced that this is a bugfix (as opposed to "add a
new feature to please Luke while regressing it for others"), but the
"this looks too invasive" made me look at the codepath involved.
There is this code in daemon.c::path_ok() that takes "dir" and ends
up calling enter_repo().
if (*dir == '~') {
if (!user_path) {
logerror("'%s': User-path not allowed", dir);
return NULL;
}
At first glance, it ought to be a single-liner
- if (*dir == '~') {
+ if (user_path && *dir == '~') {
to make 'git://site/~foo.git" go to "/var/scm/~foo.git" when base
path is set to /var/scm/.
Unfortunately that is not sufficient.
A request to "git://<site>/<string>", depending on <string>, results
in "directory" given to path_ok() in a bit different forms. Namely,
connect.c::parse_connect_url() gives
URL directory
git://site/nouser.git /nouser.git
git://site/~nouser.git ~nouser.git
by special casing "~user" syntax (in other words, a pathname that
begins with a tilde _is_ special cased, and tilde is not considered
a normal character that can be in a pathname). Note the lack of
leading slash in the second one.
Because that is how the shipped clients work, the daemon needs a bit
of adjustment, because interpolation and base-path prefix codepaths
wants to accept only paths that begin with a slash, and relies on
the slash when interpolating it in the template or concatenating it
to the base (e.g. roughly "sprintf(buf, "%s%s", base_path, dir)").
I came up with the following as a less invasive patch. There no
longer is the ase where "user-path not allowed" error is given,
as treating tilde as just a normal pathname character even when it
appears at the beginning is the whole point of this change.
As I said already, I am not sure if this is a good change, though.
daemon.c | 9 +++++----
t/t5570-git-daemon.sh | 11 +++++++++++
2 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/daemon.c b/daemon.c
index afce1b9b7f..e560a535af 100644
--- a/daemon.c
+++ b/daemon.c
@@ -160,6 +160,7 @@ static const char *path_ok(const char *directory, struct hostinfo *hi)
{
static char rpath[PATH_MAX];
static char interp_path[PATH_MAX];
+ char tilde_path[PATH_MAX];
const char *path;
const char *dir;
@@ -170,11 +171,11 @@ static const char *path_ok(const char *directory, struct hostinfo *hi)
return NULL;
}
+ if (!user_path && *dir == '~') {
+ snprintf(tilde_path, PATH_MAX, "/%s", dir);
+ dir = tilde_path;
+ }
if (*dir == '~') {
- if (!user_path) {
- logerror("'%s': User-path not allowed", dir);
- return NULL;
- }
if (*user_path) {
/* Got either "~alice" or "~alice/foo";
* rewrite them to "~alice/%s" or
diff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh
index 225a022e8a..b6d2b9a001 100755
--- a/t/t5570-git-daemon.sh
+++ b/t/t5570-git-daemon.sh
@@ -65,6 +65,17 @@ test_expect_success 'remote detects correct HEAD' '
)
'
+test_expect_success 'tilde is a normal character without --user-path' '
+ nouser="~nouser.git" &&
+ nouser_repo="$GIT_DAEMON_DOCUMENT_ROOT_PATH/$nouser" &&
+ mkdir "$nouser_repo" &&
+ git -C "$nouser_repo" --bare init &&
+ >"$nouser_repo/git-daemon-export-ok" &&
+ git push "$nouser_repo" master:master &&
+
+ git ls-remote "$GIT_DAEMON_URL/$nouser"
+'
+
test_expect_success 'prepare pack objects' '
cp -R "$GIT_DAEMON_DOCUMENT_ROOT_PATH"/repo.git "$GIT_DAEMON_DOCUMENT_ROOT_PATH"/repo_pack.git &&
(cd "$GIT_DAEMON_DOCUMENT_ROOT_PATH"/repo_pack.git &&
^ permalink raw reply related
* RFC Failover url for fetches?
From: Stefan Beller @ 2016-10-21 18:19 UTC (permalink / raw)
To: git@vger.kernel.org
So when pushing it is possible to have multiple urls
(remote.<name>.url) configured.
When fetching only the first configured url is considered.
Would it make sense to allow multiple urls and
try them one by one until one works?
Background:
http://www.businessinsider.com/amazon-spotify-twitter-github-and-etsy-down-in-apparent-dns-attack-2016-10
I wanted to fetch and had to fix a url of a repo
to point to a working mirror.
Thanks,
Stefan
^ permalink raw reply
* [PATCH v5 2/8] trailer: use list.h for doubly-linked list
From: Jonathan Tan @ 2016-10-21 17:54 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, sbeller, gitster, Ramsay Jones
In-Reply-To: <cover.1477072247.git.jonathantanmy@google.com>
Replace the existing handwritten implementation of a doubly-linked list
in trailer.c with the functions and macros from list.h. This
significantly simplifies the code.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
trailer.c | 258 ++++++++++++++++++++++----------------------------------------
1 file changed, 91 insertions(+), 167 deletions(-)
diff --git a/trailer.c b/trailer.c
index 1f191b2..4e85aae 100644
--- a/trailer.c
+++ b/trailer.c
@@ -4,6 +4,7 @@
#include "commit.h"
#include "tempfile.h"
#include "trailer.h"
+#include "list.h"
/*
* Copyright (c) 2013, 2014 Christian Couder <chriscool@tuxfamily.org>
*/
@@ -25,19 +26,24 @@ struct conf_info {
static struct conf_info default_conf_info;
struct trailer_item {
- struct trailer_item *previous;
- struct trailer_item *next;
+ struct list_head list;
char *token;
char *value;
struct conf_info conf;
};
-static struct trailer_item *first_conf_item;
+static LIST_HEAD(conf_head);
static char *separators = ":";
#define TRAILER_ARG_STRING "$ARG"
+/* Iterate over the elements of the list. */
+#define list_for_each_dir(pos, head, is_reverse) \
+ for (pos = is_reverse ? (head)->prev : (head)->next; \
+ pos != (head); \
+ pos = is_reverse ? pos->prev : pos->next)
+
static int after_or_end(enum action_where where)
{
return (where == WHERE_AFTER) || (where == WHERE_END);
@@ -120,101 +126,49 @@ static void print_tok_val(FILE *outfile, const char *tok, const char *val)
fprintf(outfile, "%s%c %s\n", tok, separators[0], val);
}
-static void print_all(FILE *outfile, struct trailer_item *first, int trim_empty)
+static void print_all(FILE *outfile, struct list_head *head, int trim_empty)
{
+ struct list_head *pos;
struct trailer_item *item;
- for (item = first; item; item = item->next) {
+ list_for_each(pos, head) {
+ item = list_entry(pos, struct trailer_item, list);
if (!trim_empty || strlen(item->value) > 0)
print_tok_val(outfile, item->token, item->value);
}
}
-static void update_last(struct trailer_item **last)
-{
- if (*last)
- while ((*last)->next != NULL)
- *last = (*last)->next;
-}
-
-static void update_first(struct trailer_item **first)
-{
- if (*first)
- while ((*first)->previous != NULL)
- *first = (*first)->previous;
-}
-
static void add_arg_to_input_list(struct trailer_item *on_tok,
- struct trailer_item *arg_tok,
- struct trailer_item **first,
- struct trailer_item **last)
-{
- if (after_or_end(arg_tok->conf.where)) {
- arg_tok->next = on_tok->next;
- on_tok->next = arg_tok;
- arg_tok->previous = on_tok;
- if (arg_tok->next)
- arg_tok->next->previous = arg_tok;
- update_last(last);
- } else {
- arg_tok->previous = on_tok->previous;
- on_tok->previous = arg_tok;
- arg_tok->next = on_tok;
- if (arg_tok->previous)
- arg_tok->previous->next = arg_tok;
- update_first(first);
- }
+ struct trailer_item *arg_tok)
+{
+ if (after_or_end(arg_tok->conf.where))
+ list_add(&arg_tok->list, &on_tok->list);
+ else
+ list_add_tail(&arg_tok->list, &on_tok->list);
}
static int check_if_different(struct trailer_item *in_tok,
struct trailer_item *arg_tok,
- int check_all)
+ int check_all,
+ struct list_head *head)
{
enum action_where where = arg_tok->conf.where;
+ struct list_head *next_head;
do {
- if (!in_tok)
- return 1;
if (same_trailer(in_tok, arg_tok))
return 0;
/*
* if we want to add a trailer after another one,
* we have to check those before this one
*/
- in_tok = after_or_end(where) ? in_tok->previous : in_tok->next;
+ next_head = after_or_end(where) ? in_tok->list.prev
+ : in_tok->list.next;
+ if (next_head == head)
+ break;
+ in_tok = list_entry(next_head, struct trailer_item, list);
} while (check_all);
return 1;
}
-static void remove_from_list(struct trailer_item *item,
- struct trailer_item **first,
- struct trailer_item **last)
-{
- struct trailer_item *next = item->next;
- struct trailer_item *previous = item->previous;
-
- if (next) {
- item->next->previous = previous;
- item->next = NULL;
- } else if (last)
- *last = previous;
-
- if (previous) {
- item->previous->next = next;
- item->previous = NULL;
- } else if (first)
- *first = next;
-}
-
-static struct trailer_item *remove_first(struct trailer_item **first)
-{
- struct trailer_item *item = *first;
- *first = item->next;
- if (item->next) {
- item->next->previous = NULL;
- item->next = NULL;
- }
- return item;
-}
-
static char *apply_command(const char *command, const char *arg)
{
struct strbuf cmd = STRBUF_INIT;
@@ -266,8 +220,7 @@ static void apply_item_command(struct trailer_item *in_tok, struct trailer_item
static void apply_arg_if_exists(struct trailer_item *in_tok,
struct trailer_item *arg_tok,
struct trailer_item *on_tok,
- struct trailer_item **in_tok_first,
- struct trailer_item **in_tok_last)
+ struct list_head *head)
{
switch (arg_tok->conf.if_exists) {
case EXISTS_DO_NOTHING:
@@ -275,40 +228,34 @@ static void apply_arg_if_exists(struct trailer_item *in_tok,
break;
case EXISTS_REPLACE:
apply_item_command(in_tok, arg_tok);
- add_arg_to_input_list(on_tok, arg_tok,
- in_tok_first, in_tok_last);
- remove_from_list(in_tok, in_tok_first, in_tok_last);
+ add_arg_to_input_list(on_tok, arg_tok);
+ list_del(&in_tok->list);
free_trailer_item(in_tok);
break;
case EXISTS_ADD:
apply_item_command(in_tok, arg_tok);
- add_arg_to_input_list(on_tok, arg_tok,
- in_tok_first, in_tok_last);
+ add_arg_to_input_list(on_tok, arg_tok);
break;
case EXISTS_ADD_IF_DIFFERENT:
apply_item_command(in_tok, arg_tok);
- if (check_if_different(in_tok, arg_tok, 1))
- add_arg_to_input_list(on_tok, arg_tok,
- in_tok_first, in_tok_last);
+ if (check_if_different(in_tok, arg_tok, 1, head))
+ add_arg_to_input_list(on_tok, arg_tok);
else
free_trailer_item(arg_tok);
break;
case EXISTS_ADD_IF_DIFFERENT_NEIGHBOR:
apply_item_command(in_tok, arg_tok);
- if (check_if_different(on_tok, arg_tok, 0))
- add_arg_to_input_list(on_tok, arg_tok,
- in_tok_first, in_tok_last);
+ if (check_if_different(on_tok, arg_tok, 0, head))
+ add_arg_to_input_list(on_tok, arg_tok);
else
free_trailer_item(arg_tok);
break;
}
}
-static void apply_arg_if_missing(struct trailer_item **in_tok_first,
- struct trailer_item **in_tok_last,
+static void apply_arg_if_missing(struct list_head *head,
struct trailer_item *arg_tok)
{
- struct trailer_item **in_tok;
enum action_where where;
switch (arg_tok->conf.if_missing) {
@@ -317,68 +264,60 @@ static void apply_arg_if_missing(struct trailer_item **in_tok_first,
break;
case MISSING_ADD:
where = arg_tok->conf.where;
- in_tok = after_or_end(where) ? in_tok_last : in_tok_first;
apply_item_command(NULL, arg_tok);
- if (*in_tok) {
- add_arg_to_input_list(*in_tok, arg_tok,
- in_tok_first, in_tok_last);
- } else {
- *in_tok_first = arg_tok;
- *in_tok_last = arg_tok;
- }
- break;
+ if (after_or_end(where))
+ list_add_tail(&arg_tok->list, head);
+ else
+ list_add(&arg_tok->list, head);
}
}
-static int find_same_and_apply_arg(struct trailer_item **in_tok_first,
- struct trailer_item **in_tok_last,
+static int find_same_and_apply_arg(struct list_head *head,
struct trailer_item *arg_tok)
{
+ struct list_head *pos;
struct trailer_item *in_tok;
struct trailer_item *on_tok;
- struct trailer_item *following_tok;
enum action_where where = arg_tok->conf.where;
int middle = (where == WHERE_AFTER) || (where == WHERE_BEFORE);
int backwards = after_or_end(where);
- struct trailer_item *start_tok = backwards ? *in_tok_last : *in_tok_first;
+ struct trailer_item *start_tok;
- for (in_tok = start_tok; in_tok; in_tok = following_tok) {
- following_tok = backwards ? in_tok->previous : in_tok->next;
+ if (list_empty(head))
+ return 0;
+
+ start_tok = list_entry(backwards ? head->prev : head->next,
+ struct trailer_item,
+ list);
+
+ list_for_each_dir(pos, head, backwards) {
+ in_tok = list_entry(pos, struct trailer_item, list);
if (!same_token(in_tok, arg_tok))
continue;
on_tok = middle ? in_tok : start_tok;
- apply_arg_if_exists(in_tok, arg_tok, on_tok,
- in_tok_first, in_tok_last);
+ apply_arg_if_exists(in_tok, arg_tok, on_tok, head);
return 1;
}
return 0;
}
-static void process_trailers_lists(struct trailer_item **in_tok_first,
- struct trailer_item **in_tok_last,
- struct trailer_item **arg_tok_first)
+static void process_trailers_lists(struct list_head *head,
+ struct list_head *arg_head)
{
+ struct list_head *pos, *p;
struct trailer_item *arg_tok;
- struct trailer_item *next_arg;
-
- if (!*arg_tok_first)
- return;
- for (arg_tok = *arg_tok_first; arg_tok; arg_tok = next_arg) {
+ list_for_each_safe(pos, p, arg_head) {
int applied = 0;
+ arg_tok = list_entry(pos, struct trailer_item, list);
- next_arg = arg_tok->next;
- remove_from_list(arg_tok, arg_tok_first, NULL);
+ list_del(pos);
- applied = find_same_and_apply_arg(in_tok_first,
- in_tok_last,
- arg_tok);
+ applied = find_same_and_apply_arg(head, arg_tok);
if (!applied)
- apply_arg_if_missing(in_tok_first,
- in_tok_last,
- arg_tok);
+ apply_arg_if_missing(head, arg_tok);
}
}
@@ -438,13 +377,12 @@ static void duplicate_conf(struct conf_info *dst, const struct conf_info *src)
static struct trailer_item *get_conf_item(const char *name)
{
+ struct list_head *pos;
struct trailer_item *item;
- struct trailer_item *previous;
/* Look up item with same name */
- for (previous = NULL, item = first_conf_item;
- item;
- previous = item, item = item->next) {
+ list_for_each(pos, &conf_head) {
+ item = list_entry(pos, struct trailer_item, list);
if (!strcasecmp(item->conf.name, name))
return item;
}
@@ -454,12 +392,7 @@ static struct trailer_item *get_conf_item(const char *name)
duplicate_conf(&item->conf, &default_conf_info);
item->conf.name = xstrdup(name);
- if (!previous)
- first_conf_item = item;
- else {
- previous->next = item;
- item->previous = previous;
- }
+ list_add_tail(&item->list, &conf_head);
return item;
}
@@ -633,6 +566,7 @@ static struct trailer_item *create_trailer_item(const char *string)
struct strbuf val = STRBUF_INIT;
struct trailer_item *item;
int tok_len;
+ struct list_head *pos;
if (parse_trailer(&tok, &val, string))
return NULL;
@@ -640,7 +574,8 @@ static struct trailer_item *create_trailer_item(const char *string)
tok_len = token_len_without_separator(tok.buf, tok.len);
/* Lookup if the token matches something in the config */
- for (item = first_conf_item; item; item = item->next) {
+ list_for_each(pos, &conf_head) {
+ item = list_entry(pos, struct trailer_item, list);
if (token_matches_item(tok.buf, item, tok_len))
return new_trailer_item(item,
strbuf_detach(&tok, NULL),
@@ -652,44 +587,34 @@ static struct trailer_item *create_trailer_item(const char *string)
strbuf_detach(&val, NULL));
}
-static void add_trailer_item(struct trailer_item **first,
- struct trailer_item **last,
- struct trailer_item *new)
+static void add_trailer_item(struct list_head *head, struct trailer_item *new)
{
if (!new)
return;
- if (!*last) {
- *first = new;
- *last = new;
- } else {
- (*last)->next = new;
- new->previous = *last;
- *last = new;
- }
+ list_add_tail(&new->list, head);
}
-static struct trailer_item *process_command_line_args(struct string_list *trailers)
+static void process_command_line_args(struct list_head *arg_head,
+ struct string_list *trailers)
{
- struct trailer_item *arg_tok_first = NULL;
- struct trailer_item *arg_tok_last = NULL;
struct string_list_item *tr;
struct trailer_item *item;
+ struct list_head *pos;
/* Add a trailer item for each configured trailer with a command */
- for (item = first_conf_item; item; item = item->next) {
+ list_for_each(pos, &conf_head) {
+ item = list_entry(pos, struct trailer_item, list);
if (item->conf.command) {
struct trailer_item *new = new_trailer_item(item, NULL, NULL);
- add_trailer_item(&arg_tok_first, &arg_tok_last, new);
+ add_trailer_item(arg_head, new);
}
}
/* Add a trailer item for each trailer on the command line */
for_each_string_list_item(tr, trailers) {
struct trailer_item *new = create_trailer_item(tr->string);
- add_trailer_item(&arg_tok_first, &arg_tok_last, new);
+ add_trailer_item(arg_head, new);
}
-
- return arg_tok_first;
}
static struct strbuf **read_input_file(const char *file)
@@ -805,8 +730,7 @@ static void print_lines(FILE *outfile, struct strbuf **lines, int start, int end
static int process_input_file(FILE *outfile,
struct strbuf **lines,
- struct trailer_item **in_tok_first,
- struct trailer_item **in_tok_last)
+ struct list_head *head)
{
int count = 0;
int patch_start, trailer_start, trailer_end, i;
@@ -829,18 +753,19 @@ static int process_input_file(FILE *outfile,
for (i = trailer_start; i < trailer_end; i++) {
if (lines[i]->buf[0] != comment_line_char) {
struct trailer_item *new = create_trailer_item(lines[i]->buf);
- add_trailer_item(in_tok_first, in_tok_last, new);
+ add_trailer_item(head, new);
}
}
return trailer_end;
}
-static void free_all(struct trailer_item **first)
+static void free_all(struct list_head *head)
{
- while (*first) {
- struct trailer_item *item = remove_first(first);
- free_trailer_item(item);
+ struct list_head *pos, *p;
+ list_for_each_safe(pos, p, head) {
+ list_del(pos);
+ free_trailer_item(list_entry(pos, struct trailer_item, list));
}
}
@@ -877,9 +802,8 @@ static FILE *create_in_place_tempfile(const char *file)
void process_trailers(const char *file, int in_place, int trim_empty, struct string_list *trailers)
{
- struct trailer_item *in_tok_first = NULL;
- struct trailer_item *in_tok_last = NULL;
- struct trailer_item *arg_tok_first;
+ LIST_HEAD(head);
+ LIST_HEAD(arg_head);
struct strbuf **lines;
int trailer_end;
FILE *outfile = stdout;
@@ -894,15 +818,15 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str
outfile = create_in_place_tempfile(file);
/* Print the lines before the trailers */
- trailer_end = process_input_file(outfile, lines, &in_tok_first, &in_tok_last);
+ trailer_end = process_input_file(outfile, lines, &head);
- arg_tok_first = process_command_line_args(trailers);
+ process_command_line_args(&arg_head, trailers);
- process_trailers_lists(&in_tok_first, &in_tok_last, &arg_tok_first);
+ process_trailers_lists(&head, &arg_head);
- print_all(outfile, in_tok_first, trim_empty);
+ print_all(outfile, &head, trim_empty);
- free_all(&in_tok_first);
+ free_all(&head);
/* Print the lines after the trailers as is */
print_lines(outfile, lines, trailer_end, INT_MAX);
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v5 3/8] trailer: streamline trailer item create and add
From: Jonathan Tan @ 2016-10-21 17:54 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, sbeller, gitster
In-Reply-To: <cover.1477072247.git.jonathantanmy@google.com>
Currently, creation and addition (to a list) of trailer items are spread
across multiple functions. Streamline this by only having 2 functions:
one to parse the user-supplied string, and one to add the parsed
information to a list.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
trailer.c | 130 +++++++++++++++++++++++++++++---------------------------------
1 file changed, 60 insertions(+), 70 deletions(-)
diff --git a/trailer.c b/trailer.c
index 4e85aae..ae3972a 100644
--- a/trailer.c
+++ b/trailer.c
@@ -500,10 +500,31 @@ static int git_trailer_config(const char *conf_key, const char *value, void *cb)
return 0;
}
-static int parse_trailer(struct strbuf *tok, struct strbuf *val, const char *trailer)
+static const char *token_from_item(struct trailer_item *item, char *tok)
+{
+ if (item->conf.key)
+ return item->conf.key;
+ if (tok)
+ return tok;
+ return item->conf.name;
+}
+
+static int token_matches_item(const char *tok, struct trailer_item *item, int tok_len)
+{
+ if (!strncasecmp(tok, item->conf.name, tok_len))
+ return 1;
+ return item->conf.key ? !strncasecmp(tok, item->conf.key, tok_len) : 0;
+}
+
+static int parse_trailer(struct strbuf *tok, struct strbuf *val,
+ const struct conf_info **conf, const char *trailer)
{
size_t len;
struct strbuf seps = STRBUF_INIT;
+ struct trailer_item *item;
+ int tok_len;
+ struct list_head *pos;
+
strbuf_addstr(&seps, separators);
strbuf_addch(&seps, '=');
len = strcspn(trailer, seps.buf);
@@ -523,74 +544,31 @@ static int parse_trailer(struct strbuf *tok, struct strbuf *val, const char *tra
strbuf_addstr(tok, trailer);
strbuf_trim(tok);
}
- return 0;
-}
-
-static const char *token_from_item(struct trailer_item *item, char *tok)
-{
- if (item->conf.key)
- return item->conf.key;
- if (tok)
- return tok;
- return item->conf.name;
-}
-
-static struct trailer_item *new_trailer_item(struct trailer_item *conf_item,
- char *tok, char *val)
-{
- struct trailer_item *new = xcalloc(sizeof(*new), 1);
- new->value = val ? val : xstrdup("");
-
- if (conf_item) {
- duplicate_conf(&new->conf, &conf_item->conf);
- new->token = xstrdup(token_from_item(conf_item, tok));
- free(tok);
- } else {
- duplicate_conf(&new->conf, &default_conf_info);
- new->token = tok;
- }
-
- return new;
-}
-
-static int token_matches_item(const char *tok, struct trailer_item *item, int tok_len)
-{
- if (!strncasecmp(tok, item->conf.name, tok_len))
- return 1;
- return item->conf.key ? !strncasecmp(tok, item->conf.key, tok_len) : 0;
-}
-
-static struct trailer_item *create_trailer_item(const char *string)
-{
- struct strbuf tok = STRBUF_INIT;
- struct strbuf val = STRBUF_INIT;
- struct trailer_item *item;
- int tok_len;
- struct list_head *pos;
-
- if (parse_trailer(&tok, &val, string))
- return NULL;
-
- tok_len = token_len_without_separator(tok.buf, tok.len);
/* Lookup if the token matches something in the config */
+ tok_len = token_len_without_separator(tok->buf, tok->len);
+ *conf = &default_conf_info;
list_for_each(pos, &conf_head) {
item = list_entry(pos, struct trailer_item, list);
- if (token_matches_item(tok.buf, item, tok_len))
- return new_trailer_item(item,
- strbuf_detach(&tok, NULL),
- strbuf_detach(&val, NULL));
+ if (token_matches_item(tok->buf, item, tok_len)) {
+ char *tok_buf = strbuf_detach(tok, NULL);
+ *conf = &item->conf;
+ strbuf_addstr(tok, token_from_item(item, tok_buf));
+ free(tok_buf);
+ break;
+ }
}
- return new_trailer_item(NULL,
- strbuf_detach(&tok, NULL),
- strbuf_detach(&val, NULL));
+ return 0;
}
-static void add_trailer_item(struct list_head *head, struct trailer_item *new)
+static void add_trailer_item(struct list_head *head, char *tok, char *val,
+ const struct conf_info *conf)
{
- if (!new)
- return;
+ struct trailer_item *new = xcalloc(sizeof(*new), 1);
+ new->token = tok;
+ new->value = val;
+ duplicate_conf(&new->conf, conf);
list_add_tail(&new->list, head);
}
@@ -599,21 +577,28 @@ static void process_command_line_args(struct list_head *arg_head,
{
struct string_list_item *tr;
struct trailer_item *item;
+ struct strbuf tok = STRBUF_INIT;
+ struct strbuf val = STRBUF_INIT;
+ const struct conf_info *conf;
struct list_head *pos;
/* Add a trailer item for each configured trailer with a command */
list_for_each(pos, &conf_head) {
item = list_entry(pos, struct trailer_item, list);
- if (item->conf.command) {
- struct trailer_item *new = new_trailer_item(item, NULL, NULL);
- add_trailer_item(arg_head, new);
- }
+ if (item->conf.command)
+ add_trailer_item(arg_head,
+ xstrdup(token_from_item(item, NULL)),
+ xstrdup(""),
+ &item->conf);
}
/* Add a trailer item for each trailer on the command line */
for_each_string_list_item(tr, trailers) {
- struct trailer_item *new = create_trailer_item(tr->string);
- add_trailer_item(arg_head, new);
+ if (!parse_trailer(&tok, &val, &conf, tr->string))
+ add_trailer_item(arg_head,
+ strbuf_detach(&tok, NULL),
+ strbuf_detach(&val, NULL),
+ conf);
}
}
@@ -734,6 +719,9 @@ static int process_input_file(FILE *outfile,
{
int count = 0;
int patch_start, trailer_start, trailer_end, i;
+ struct strbuf tok = STRBUF_INIT;
+ struct strbuf val = STRBUF_INIT;
+ const struct conf_info *conf;
/* Get the line count */
while (lines[count])
@@ -751,10 +739,12 @@ static int process_input_file(FILE *outfile,
/* Parse trailer lines */
for (i = trailer_start; i < trailer_end; i++) {
- if (lines[i]->buf[0] != comment_line_char) {
- struct trailer_item *new = create_trailer_item(lines[i]->buf);
- add_trailer_item(head, new);
- }
+ if (lines[i]->buf[0] != comment_line_char &&
+ !parse_trailer(&tok, &val, &conf, lines[i]->buf))
+ add_trailer_item(head,
+ strbuf_detach(&tok, NULL),
+ strbuf_detach(&val, NULL),
+ conf);
}
return trailer_end;
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v5 8/8] trailer: support values folded to multiple lines
From: Jonathan Tan @ 2016-10-21 17:55 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, sbeller, gitster
In-Reply-To: <cover.1477072247.git.jonathantanmy@google.com>
Currently, interpret-trailers requires that a trailer be only on 1 line.
For example:
a: first line
second line
would be interpreted as one trailer line followed by one non-trailer line.
Make interpret-trailers support RFC 822-style folding, treating those
lines as one logical trailer.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
Documentation/git-interpret-trailers.txt | 7 +-
t/t7513-interpret-trailers.sh | 169 +++++++++++++++++++++++++++++++
trailer.c | 45 ++++++--
3 files changed, 211 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt
index 4966b5b..e99bda6 100644
--- a/Documentation/git-interpret-trailers.txt
+++ b/Documentation/git-interpret-trailers.txt
@@ -57,11 +57,12 @@ minus signs start the patch part of the message.
When reading trailers, there can be whitespaces after the
token, the separator and the value. There can also be whitespaces
-inside the token and the value.
+inside the token and the value. The value may be split over multiple lines with
+each subsequent line starting with whitespace, like the "folding" in RFC 822.
Note that 'trailers' do not follow and are not intended to follow many
-rules for RFC 822 headers. For example they do not follow the line
-folding rules, the encoding rules and probably many other rules.
+rules for RFC 822 headers. For example they do not follow
+the encoding rules and probably many other rules.
OPTIONS
-------
diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index 3d94b3a..4dd1d7c 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -256,6 +256,175 @@ test_expect_success 'line with leading whitespace is not trailer' '
test_cmp expected actual
'
+test_expect_success 'multiline field treated as one trailer for 25% check' '
+ q_to_tab >patch <<-\EOF &&
+
+ Signed-off-by: a <a@example.com>
+ name: value on
+ Qmultiple lines
+ this is not a trailer
+ this is not a trailer
+ this is not a trailer
+ this is not a trailer
+ this is not a trailer
+ this is not a trailer
+ EOF
+ q_to_tab >expected <<-\EOF &&
+
+ Signed-off-by: a <a@example.com>
+ name: value on
+ Qmultiple lines
+ this is not a trailer
+ this is not a trailer
+ this is not a trailer
+ this is not a trailer
+ this is not a trailer
+ this is not a trailer
+ name: value
+ EOF
+ git interpret-trailers --trailer "name: value" patch >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'multiline field treated as atomic for placement' '
+ q_to_tab >patch <<-\EOF &&
+
+ another: trailer
+ name: value on
+ Qmultiple lines
+ another: trailer
+ EOF
+ q_to_tab >expected <<-\EOF &&
+
+ another: trailer
+ name: value on
+ Qmultiple lines
+ name: value
+ another: trailer
+ EOF
+ test_config trailer.name.where after &&
+ git interpret-trailers --trailer "name: value" patch >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'multiline field treated as atomic for replacement' '
+ q_to_tab >patch <<-\EOF &&
+
+ another: trailer
+ name: value on
+ Qmultiple lines
+ another: trailer
+ EOF
+ q_to_tab >expected <<-\EOF &&
+
+ another: trailer
+ another: trailer
+ name: value
+ EOF
+ test_config trailer.name.ifexists replace &&
+ git interpret-trailers --trailer "name: value" patch >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'multiline field treated as atomic for difference check' '
+ q_to_tab >patch <<-\EOF &&
+
+ another: trailer
+ name: first line
+ Qsecond line
+ another: trailer
+ EOF
+ test_config trailer.name.ifexists addIfDifferent &&
+
+ q_to_tab >trailer <<-\EOF &&
+ name: first line
+ Qsecond line
+ EOF
+ q_to_tab >expected <<-\EOF &&
+
+ another: trailer
+ name: first line
+ Qsecond line
+ another: trailer
+ EOF
+ git interpret-trailers --trailer "$(cat trailer)" patch >actual &&
+ test_cmp expected actual &&
+
+ q_to_tab >trailer <<-\EOF &&
+ name: first line
+ QQQQQsecond line
+ EOF
+ q_to_tab >expected <<-\EOF &&
+
+ another: trailer
+ name: first line
+ Qsecond line
+ another: trailer
+ name: first line
+ QQQQQsecond line
+ EOF
+ git interpret-trailers --trailer "$(cat trailer)" patch >actual &&
+ test_cmp expected actual &&
+
+ q_to_tab >trailer <<-\EOF &&
+ name: first line *DIFFERENT*
+ Qsecond line
+ EOF
+ q_to_tab >expected <<-\EOF &&
+
+ another: trailer
+ name: first line
+ Qsecond line
+ another: trailer
+ name: first line *DIFFERENT*
+ Qsecond line
+ EOF
+ git interpret-trailers --trailer "$(cat trailer)" patch >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'multiline field treated as atomic for neighbor check' '
+ q_to_tab >patch <<-\EOF &&
+
+ another: trailer
+ name: first line
+ Qsecond line
+ another: trailer
+ EOF
+ test_config trailer.name.where after &&
+ test_config trailer.name.ifexists addIfDifferentNeighbor &&
+
+ q_to_tab >trailer <<-\EOF &&
+ name: first line
+ Qsecond line
+ EOF
+ q_to_tab >expected <<-\EOF &&
+
+ another: trailer
+ name: first line
+ Qsecond line
+ another: trailer
+ EOF
+ git interpret-trailers --trailer "$(cat trailer)" patch >actual &&
+ test_cmp expected actual &&
+
+ q_to_tab >trailer <<-\EOF &&
+ name: first line
+ QQQQQsecond line
+ EOF
+ q_to_tab >expected <<-\EOF &&
+
+ another: trailer
+ name: first line
+ Qsecond line
+ name: first line
+ QQQQQsecond line
+ another: trailer
+ EOF
+ git interpret-trailers --trailer "$(cat trailer)" patch >actual &&
+ test_cmp expected actual
+'
+
test_expect_success 'with config setup' '
git config trailer.ack.key "Acked-by: " &&
cat >expected <<-\EOF &&
diff --git a/trailer.c b/trailer.c
index d978437..65866d0 100644
--- a/trailer.c
+++ b/trailer.c
@@ -619,12 +619,14 @@ static void parse_trailer(struct strbuf *tok, struct strbuf *val,
}
}
-static void add_trailer_item(struct list_head *head, char *tok, char *val)
+static struct trailer_item *add_trailer_item(struct list_head *head, char *tok,
+ char *val)
{
struct trailer_item *new = xcalloc(sizeof(*new), 1);
new->token = tok;
new->value = val;
list_add_tail(&new->list, head);
+ return new;
}
static void add_arg_item(struct list_head *arg_head, char *tok, char *val,
@@ -732,6 +734,14 @@ static int find_trailer_start(struct strbuf **lines, int count)
{
int start, end_of_title, only_spaces = 1;
int recognized_prefix = 0, trailer_lines = 0, non_trailer_lines = 0;
+ /*
+ * Number of possible continuation lines encountered. This will be
+ * reset to 0 if we encounter a trailer (since those lines are to be
+ * considered continuations of that trailer), and added to
+ * non_trailer_lines if we encounter a non-trailer (since those lines
+ * are to be considered non-trailers).
+ */
+ int possible_continuation_lines = 0;
/* The first paragraph is the title and cannot be trailers */
for (start = 0; start < count; start++) {
@@ -752,11 +762,15 @@ static int find_trailer_start(struct strbuf **lines, int count)
const char **p;
int separator_pos;
- if (lines[start]->buf[0] == comment_line_char)
+ if (lines[start]->buf[0] == comment_line_char) {
+ non_trailer_lines += possible_continuation_lines;
+ possible_continuation_lines = 0;
continue;
+ }
if (contains_only_spaces(lines[start]->buf)) {
if (only_spaces)
continue;
+ non_trailer_lines += possible_continuation_lines;
if (recognized_prefix &&
trailer_lines * 3 >= non_trailer_lines)
return start + 1;
@@ -769,16 +783,18 @@ static int find_trailer_start(struct strbuf **lines, int count)
for (p = git_generated_prefixes; *p; p++) {
if (starts_with(lines[start]->buf, *p)) {
trailer_lines++;
+ possible_continuation_lines = 0;
recognized_prefix = 1;
goto continue_outer_loop;
}
}
- separator_pos = find_separator(lines[start]->buf);
+ separator_pos = find_separator(lines[start]->buf, separators);
if (separator_pos >= 1 && !isspace(lines[start]->buf[0])) {
struct list_head *pos;
trailer_lines++;
+ possible_continuation_lines = 0;
if (recognized_prefix)
continue;
list_for_each(pos, &conf_head) {
@@ -790,8 +806,13 @@ static int find_trailer_start(struct strbuf **lines, int count)
break;
}
}
- } else
+ } else if (isspace(lines[start]->buf[0]))
+ possible_continuation_lines++;
+ else {
non_trailer_lines++;
+ non_trailer_lines += possible_continuation_lines;
+ possible_continuation_lines = 0;
+ }
continue_outer_loop:
;
}
@@ -840,6 +861,7 @@ static int process_input_file(FILE *outfile,
int patch_start, trailer_start, trailer_end, i;
struct strbuf tok = STRBUF_INIT;
struct strbuf val = STRBUF_INIT;
+ struct trailer_item *last = NULL;
/* Get the line count */
while (lines[count])
@@ -860,19 +882,28 @@ static int process_input_file(FILE *outfile,
int separator_pos;
if (lines[i]->buf[0] == comment_line_char)
continue;
+ if (last && isspace(lines[i]->buf[0])) {
+ struct strbuf sb = STRBUF_INIT;
+ strbuf_addf(&sb, "%s\n%s", last->value, lines[i]->buf);
+ strbuf_strip_suffix(&sb, "\n");
+ free(last->value);
+ last->value = strbuf_detach(&sb, NULL);
+ continue;
+ }
separator_pos = find_separator(lines[i]->buf, separators);
if (separator_pos >= 1) {
parse_trailer(&tok, &val, NULL, lines[i]->buf,
separator_pos);
- add_trailer_item(head,
- strbuf_detach(&tok, NULL),
- strbuf_detach(&val, NULL));
+ last = add_trailer_item(head,
+ strbuf_detach(&tok, NULL),
+ strbuf_detach(&val, NULL));
} else {
strbuf_addbuf(&val, lines[i]);
strbuf_strip_suffix(&val, "\n");
add_trailer_item(head,
NULL,
strbuf_detach(&val, NULL));
+ last = NULL;
}
}
--
2.8.0.rc3.226.g39d4020
^ 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