* Re: [PATCH v2 2/3] gitweb: Link to 7-char+ SHA1s, not only 8-char+
From: Junio C Hamano @ 2016-10-14 18:40 UTC (permalink / raw)
To: Jakub Narębski; +Cc: Ævar Arnfjörð Bjarmason, git
In-Reply-To: <3fa9902f-3b01-5ec6-8129-34cff4c7cac9@gmail.com>
Jakub Narębski <jnareb@gmail.com> writes:
> s/SHA1/SHA-1/g in above paragraph (for correctness and consistency).
>>
>> I think it's fairly dubious to link to things matching [0-9a-fA-F]
>> here as opposed to just [0-9a-f], that dates back to the initial
>> version of gitweb from 161332a ("first working version",
>> 2005-08-07). Git will accept all-caps SHA1s, but didn't ever produce
>> them as far as I can tell.
>
> All right. If we decide to be more strict in what we accept, we can
> do it in a separate commit.
>
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>
> Acked-by: Jakub Narębski <jnareb@gmail.com>
Thanks for a review. As the topic is not yet in 'next', I'll squish
in your Acked-by: to them. I saw them only for 1 & 2/3; would
another for 3/3 be coming soon?
>
>> ---
>> gitweb/gitweb.perl | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>> index cba7405..92b5e91 100755
>> --- a/gitweb/gitweb.perl
>> +++ b/gitweb/gitweb.perl
>> @@ -2036,7 +2036,7 @@ sub format_log_line_html {
>> my $line = shift;
>>
>> $line = esc_html($line, -nbsp=>1);
>> - $line =~ s{\b([0-9a-fA-F]{8,40})\b}{
>> + $line =~ s{\b([0-9a-fA-F]{7,40})\b}{
>
> By the way, it is quite long commit message for one character change.
> Not that it is a bad thing...
>
>> $cgi->a({-href => href(action=>"object", hash=>$1),
>> -class => "text"}, $1);
>> }eg;
>>
^ permalink raw reply
* Re: [PATCH v2 2/6] trailer: use list.h for doubly-linked list
From: Junio C Hamano @ 2016-10-14 18:27 UTC (permalink / raw)
To: Jonathan Tan; +Cc: git, peff, christian.couder
In-Reply-To: <a7b9f79ea1b61b80d1177740d5d78fcca848a4f6.1476314576.git.jonathantanmy@google.com>
Jonathan Tan <jonathantanmy@google.com> writes:
> 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.
> ---
The handcrafted one in trailer.c somehow did not use the common
practice of using a doubly-linked cycle as a doubly-linked list with
a designated fake element as the pointers to the first and to the
last elements of the list (instead it used NULL as the "this is the
end in this direction" convention just like a common singly-linked
list), and this update removes the need for special cases handling
the elements at the beginning and at the end that comes from that
choice by switching to list.h macros. update_last/update_first can
go, two parameters that were passed to point at the variables for
the beginning and the end can go, the special cases for the initial
condition in add_trailer_item() can go, all thanks to this change.
Very nice.
> trailer.c | 258 ++++++++++++++++++++++----------------------------------------
> 1 file changed, 91 insertions(+), 167 deletions(-)
^ permalink raw reply
* Re: [PATCH v2 3/3] gitweb: Link to "git describe"'d commits in log messages
From: Ævar Arnfjörð Bjarmason @ 2016-10-14 17:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git, Jakub Narebski
In-Reply-To: <CACBZZX5mnCmzT3N35YKpr2t1LT-hh-H7eS-+XTjudguedJL5Zw@mail.gmail.com>
On Sun, Oct 9, 2016 at 1:20 PM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> On Thu, Oct 6, 2016 at 9:51 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>>
>>> Change the log formatting function to know about "git describe" output
>>> such as "v2.8.0-4-g867ad08", in addition to just plain "867ad08".
>>>
>>> There are still many valid refnames that we don't link to
>>> e.g. v2.10.0-rc1~2^2~1 is also a valid way to refer to
>>> v2.8.0-4-g867ad08, but I'm not supporting that with this commit,
>>> similarly it's trivially possible to create some refnames like
>>> "æ/var-gf6727b0" or which won't be picked up by this regex.
>>
>> Not a serious counter-proposal or suggestion, and certainly not an
>> objection to the patch I am responding to, but I wonder if it is so
>> bad if we made the 867ad08 into link when showing v2.8.0-4-g867ad08.
>>
>> IOW, in addition to \b followed by [0-9a-f]+ followed by \b, if we
>> allowed an optional 'g' in front of the hex, e.g.
>>
>> - $line =~ s{\b([0-9a-fA-F]{7,40})\b}{
>> + $line =~ s{\bg?([0-9a-fA-F]{7,40})\b}{
>>
>> wouldn't that be much simpler, covers more cases and sufficient?
>
> It would be more simpler, maybe that's the right approach. I have a
> preference for making the entire reference a link. I think it makes
> more sense for the UI.
I just ran into an example of a better reason for doing it like my
patch is doing, which is that if you have some tag like:
deployment-20160928-171914-16-g42e13d8
With my patch the whole thing will be a link to the 42e13d8 commit,
but with this suggestion both 20160928 and 42e13d8 would become commit
links, the former one would be broken.
Of course we could have some code that would detect that the whole \S+
is part of one thing that ends in g<commit>, but the complexity in
doing that would be equivalent or more to doing what my patch is
doing.
>>> There's surely room for improvement here, but I just wanted to address
>>> the very common case of sticking "git describe" output into commit
>>> messages without trying to link to all possible refnames, that's going
>>> to be a rather futile exercise given that this is free text, and it
>>> would be prohibitively expensive to look up whether the references in
>>> question exist in our repository.
>>>
>>> There was on-list discussion about how we could do better than this
>>> patch. Junio suggested to update parse_commits() to call a new
>>> "gitweb--helper" command which would pass each of the revision
>>> candidates through "rev-parse --verify --quiet". That would cut down
>>> on our false positives (e.g. we'll link to "deadbeef"), and also allow
>>> us to be more aggressive in selecting candidate revisions.
>>>
>>> That may be too expensive to work in practice, or it may
>>> not. Investigating that would be a good follow-up to this patch.
>>>
>>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>>> ---
>>> gitweb/gitweb.perl | 18 ++++++++++++++++--
>>> 1 file changed, 16 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>>> index 92b5e91..7cf68f0 100755
>>> --- a/gitweb/gitweb.perl
>>> +++ b/gitweb/gitweb.perl
>>> @@ -2036,10 +2036,24 @@ sub format_log_line_html {
>>> my $line = shift;
>>>
>>> $line = esc_html($line, -nbsp=>1);
>>> - $line =~ s{\b([0-9a-fA-F]{7,40})\b}{
>>> + $line =~ s{
>>> + \b
>>> + (
>>> + # The output of "git describe", e.g. v2.10.0-297-gf6727b0
>>> + # or hadoop-20160921-113441-20-g094fb7d
>>> + (?<!-) # see strbuf_check_tag_ref(). Tags can't start with -
>>> + [A-Za-z0-9.-]+
>>> + (?!\.) # refs can't end with ".", see check_refname_format()
>>> + -g[0-9a-fA-F]{7,40}
>>> + |
>>> + # Just a normal looking Git SHA1
>>> + [0-9a-fA-F]{7,40}
>>> + )
>>> + \b
>>> + }{
>>> $cgi->a({-href => href(action=>"object", hash=>$1),
>>> -class => "text"}, $1);
>>> - }eg;
>>> + }egx;
>>>
>>> return $line;
>>> }
^ permalink raw reply
* Re: [PATCH 1/3] i18n: apply: mark plural string for translation
From: Junio C Hamano @ 2016-10-14 17:55 UTC (permalink / raw)
To: Vasco Almeida
Cc: git, Jiang Xin, Ævar Arnfjörð Bjarmason,
Jean-Noël AVILA
In-Reply-To: <20161014114337.18684-1-vascomalmeida@sapo.pt>
Vasco Almeida <vascomalmeida@sapo.pt> writes:
> Mark plural string for translation using Q_().
>
> Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
> ---
Thanks for waiting (patiently) for 'master' to become ready to take
these three patches.
^ permalink raw reply
* Re: [PATCH v2 2/6] trailer: use list.h for doubly-linked list
From: Jakub Narębski @ 2016-10-14 17:29 UTC (permalink / raw)
To: Jonathan Tan, git; +Cc: Junio C Hamano, Jeff King, Christian Couder
In-Reply-To: <a7b9f79ea1b61b80d1177740d5d78fcca848a4f6.1476314576.git.jonathantanmy@google.com>
W dniu 13.10.2016 o 01:40, Jonathan Tan pisze:
> 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.
> ---
> trailer.c | 258 ++++++++++++++++++++++----------------------------------------
> 1 file changed, 91 insertions(+), 167 deletions(-)
Very nice gains!
BTW. could you sign (Signed-off-by) your patches? TIA.
Best,
--
Jakub Narębski
^ permalink raw reply
* Re: [PATCH v2 2/3] gitweb: Link to 7-char+ SHA1s, not only 8-char+
From: Jakub Narębski @ 2016-10-14 17:45 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason, git; +Cc: Junio C Hamano
In-Reply-To: <20161006091135.29590-3-avarab@gmail.com>
W dniu 06.10.2016 o 11:11, Ævar Arnfjörð Bjarmason pisze:
> Change the minimum length of an abbreviated object identifier in the
> commit message gitweb tries to turn into link from 8 hexchars to 7.
>
> This arbitrary minimum length of 8 was introduced in bfe2191 ("gitweb:
> SHA-1 in commit log message links to "object" view", 2006-12-10), but
> the default abbreviation length is 7, and has been for a long time.
>
> It's still possible to reference SHA1s down to 4 characters in length,
> see v1.7.4-1-gdce9648's MINIMUM_ABBREV, but I can't see how to make
> git actually produce that, so I doubt anyone is putting that into log
> messages in practice, but people definitely do put 7 character SHA1s
> into log messages.
That's nice explanation why we want to support 7 character SHA-1
(it is the default, and was seen in the wild), but don't need to
support shorter lengths.
Another reason (just for completeness; you don't need to put it in
the commit message) to not go below 7 characters is that with
decreasing length there is more and more chance for false positives
(like 'beef' or 'caffee' for 4-char or 5-char hexstring), as I wrote
previously.
s/SHA1/SHA-1/g in above paragraph (for correctness and consistency).
>
> I think it's fairly dubious to link to things matching [0-9a-fA-F]
> here as opposed to just [0-9a-f], that dates back to the initial
> version of gitweb from 161332a ("first working version",
> 2005-08-07). Git will accept all-caps SHA1s, but didn't ever produce
> them as far as I can tell.
All right. If we decide to be more strict in what we accept, we can
do it in a separate commit.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Acked-by: Jakub Narębski <jnareb@gmail.com>
> ---
> gitweb/gitweb.perl | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index cba7405..92b5e91 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -2036,7 +2036,7 @@ sub format_log_line_html {
> my $line = shift;
>
> $line = esc_html($line, -nbsp=>1);
> - $line =~ s{\b([0-9a-fA-F]{8,40})\b}{
> + $line =~ s{\b([0-9a-fA-F]{7,40})\b}{
By the way, it is quite long commit message for one character change.
Not that it is a bad thing...
> $cgi->a({-href => href(action=>"object", hash=>$1),
> -class => "text"}, $1);
> }eg;
>
^ permalink raw reply
* Re: [PATCH] fetch: use "quick" has_sha1_file for tag following
From: Junio C Hamano @ 2016-10-14 17:39 UTC (permalink / raw)
To: Jeff King
Cc: Vegard Nossum, git, Quentin Casasnovas, Shawn Pearce,
Nguyễn Thái Ngọc Duy
In-Reply-To: <20161013200644.lnustevmpvufbg5y@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> So it's certainly better. But 7500 packs is just silly, and squeezing
> out ~400ms there is hardly worth it. If you repack this same case into a
> single pack, the command drops to 5ms. So yes, there's close to an order
> of magnitude speedup here, but you get that _and_ another order of
> magnitude just by repacking.
"7500 is silly" equally applies to the "quick" (and sloppy, if I am
reading your "Failing in this direction doesn't make me feel great."
correctly) approach, I think, which argues for not taking either
change X-<.
I agree that the fallout from the inaccuracy of "quick" approach is
probably acceptable and the next "fetch" will correct it anyway, so
let's do the "quick but inaccurate" for now and perhaps cook it in
'next' for a bit longer than other topics?
^ permalink raw reply
* [PATCH v3 4/6] trailer: make args have their own struct
From: Jonathan Tan @ 2016-10-14 17:38 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, gitster, jnareb
In-Reply-To: <cover.1476466609.git.jonathantanmy@google.com>
Improve type safety by making arguments (whether from configuration or
from the command line) have their own "struct arg_item" type, separate
from the "struct trailer_item" type used to represent the trailers in
the buffer being manipulated.
This change also prepares "struct trailer_item" to be further
differentiated from "struct arg_item" in future patches.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
trailer.c | 135 +++++++++++++++++++++++++++++++++++++++-----------------------
1 file changed, 85 insertions(+), 50 deletions(-)
diff --git a/trailer.c b/trailer.c
index 54cc930..a9ed3f8 100644
--- a/trailer.c
+++ b/trailer.c
@@ -29,6 +29,12 @@ struct trailer_item {
struct list_head list;
char *token;
char *value;
+};
+
+struct arg_item {
+ struct list_head list;
+ char *token;
+ char *value;
struct conf_info conf;
};
@@ -62,7 +68,7 @@ static size_t token_len_without_separator(const char *token, size_t len)
return len;
}
-static int same_token(struct trailer_item *a, struct trailer_item *b)
+static int same_token(struct trailer_item *a, struct arg_item *b)
{
size_t a_len = token_len_without_separator(a->token, strlen(a->token));
size_t b_len = token_len_without_separator(b->token, strlen(b->token));
@@ -71,12 +77,12 @@ static int same_token(struct trailer_item *a, struct trailer_item *b)
return !strncasecmp(a->token, b->token, min_len);
}
-static int same_value(struct trailer_item *a, struct trailer_item *b)
+static int same_value(struct trailer_item *a, struct arg_item *b)
{
return !strcasecmp(a->value, b->value);
}
-static int same_trailer(struct trailer_item *a, struct trailer_item *b)
+static int same_trailer(struct trailer_item *a, struct arg_item *b)
{
return same_token(a, b) && same_value(a, b);
}
@@ -98,6 +104,13 @@ static inline void strbuf_replace(struct strbuf *sb, const char *a, const char *
static void free_trailer_item(struct trailer_item *item)
{
+ free(item->token);
+ free(item->value);
+ free(item);
+}
+
+static void free_arg_item(struct arg_item *item)
+{
free(item->conf.name);
free(item->conf.key);
free(item->conf.command);
@@ -137,17 +150,29 @@ static void print_all(FILE *outfile, struct list_head *head, int trim_empty)
}
}
+static struct trailer_item *trailer_from_arg(struct arg_item *arg_tok)
+{
+ struct trailer_item *new = xcalloc(sizeof(*new), 1);
+ new->token = arg_tok->token;
+ new->value = arg_tok->value;
+ arg_tok->token = arg_tok->value = NULL;
+ free_arg_item(arg_tok);
+ return new;
+}
+
static void add_arg_to_input_list(struct trailer_item *on_tok,
- struct trailer_item *arg_tok)
+ struct arg_item *arg_tok)
{
- if (after_or_end(arg_tok->conf.where))
- list_add(&arg_tok->list, &on_tok->list);
+ int aoe = after_or_end(arg_tok->conf.where);
+ struct trailer_item *to_add = trailer_from_arg(arg_tok);
+ if (aoe)
+ list_add(&to_add->list, &on_tok->list);
else
- list_add_tail(&arg_tok->list, &on_tok->list);
+ list_add_tail(&to_add->list, &on_tok->list);
}
static int check_if_different(struct trailer_item *in_tok,
- struct trailer_item *arg_tok,
+ struct arg_item *arg_tok,
int check_all,
struct list_head *head)
{
@@ -200,7 +225,7 @@ static char *apply_command(const char *command, const char *arg)
return result;
}
-static void apply_item_command(struct trailer_item *in_tok, struct trailer_item *arg_tok)
+static void apply_item_command(struct trailer_item *in_tok, struct arg_item *arg_tok)
{
if (arg_tok->conf.command) {
const char *arg;
@@ -218,13 +243,13 @@ 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 arg_item *arg_tok,
struct trailer_item *on_tok,
struct list_head *head)
{
switch (arg_tok->conf.if_exists) {
case EXISTS_DO_NOTHING:
- free_trailer_item(arg_tok);
+ free_arg_item(arg_tok);
break;
case EXISTS_REPLACE:
apply_item_command(in_tok, arg_tok);
@@ -241,39 +266,41 @@ static void apply_arg_if_exists(struct trailer_item *in_tok,
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);
+ free_arg_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, head))
add_arg_to_input_list(on_tok, arg_tok);
else
- free_trailer_item(arg_tok);
+ free_arg_item(arg_tok);
break;
}
}
static void apply_arg_if_missing(struct list_head *head,
- struct trailer_item *arg_tok)
+ struct arg_item *arg_tok)
{
enum action_where where;
+ struct trailer_item *to_add;
switch (arg_tok->conf.if_missing) {
case MISSING_DO_NOTHING:
- free_trailer_item(arg_tok);
+ free_arg_item(arg_tok);
break;
case MISSING_ADD:
where = arg_tok->conf.where;
apply_item_command(NULL, arg_tok);
+ to_add = trailer_from_arg(arg_tok);
if (after_or_end(where))
- list_add_tail(&arg_tok->list, head);
+ list_add_tail(&to_add->list, head);
else
- list_add(&arg_tok->list, head);
+ list_add(&to_add->list, head);
}
}
static int find_same_and_apply_arg(struct list_head *head,
- struct trailer_item *arg_tok)
+ struct arg_item *arg_tok)
{
struct list_head *pos;
struct trailer_item *in_tok;
@@ -306,11 +333,11 @@ 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 arg_item *arg_tok;
list_for_each_safe(pos, p, arg_head) {
int applied = 0;
- arg_tok = list_entry(pos, struct trailer_item, list);
+ arg_tok = list_entry(pos, struct arg_item, list);
list_del(pos);
@@ -375,20 +402,20 @@ static void duplicate_conf(struct conf_info *dst, const struct conf_info *src)
dst->command = xstrdup(src->command);
}
-static struct trailer_item *get_conf_item(const char *name)
+static struct arg_item *get_conf_item(const char *name)
{
struct list_head *pos;
- struct trailer_item *item;
+ struct arg_item *item;
/* Look up item with same name */
list_for_each(pos, &conf_head) {
- item = list_entry(pos, struct trailer_item, list);
+ item = list_entry(pos, struct arg_item, list);
if (!strcasecmp(item->conf.name, name))
return item;
}
/* Item does not already exists, create it */
- item = xcalloc(sizeof(struct trailer_item), 1);
+ item = xcalloc(sizeof(*item), 1);
duplicate_conf(&item->conf, &default_conf_info);
item->conf.name = xstrdup(name);
@@ -442,7 +469,7 @@ static int git_trailer_default_config(const char *conf_key, const char *value, v
static int git_trailer_config(const char *conf_key, const char *value, void *cb)
{
const char *trailer_item, *variable_name;
- struct trailer_item *item;
+ struct arg_item *item;
struct conf_info *conf;
char *name = NULL;
enum trailer_info_type type;
@@ -500,7 +527,7 @@ static int git_trailer_config(const char *conf_key, const char *value, void *cb)
return 0;
}
-static const char *token_from_item(struct trailer_item *item, char *tok)
+static const char *token_from_item(struct arg_item *item, char *tok)
{
if (item->conf.key)
return item->conf.key;
@@ -509,7 +536,7 @@ static const char *token_from_item(struct trailer_item *item, char *tok)
return item->conf.name;
}
-static int token_matches_item(const char *tok, struct trailer_item *item, int tok_len)
+static int token_matches_item(const char *tok, struct arg_item *item, int tok_len)
{
if (!strncasecmp(tok, item->conf.name, tok_len))
return 1;
@@ -521,7 +548,7 @@ static int parse_trailer(struct strbuf *tok, struct strbuf *val,
{
size_t len;
struct strbuf seps = STRBUF_INIT;
- struct trailer_item *item;
+ struct arg_item *item;
int tok_len;
struct list_head *pos;
@@ -547,12 +574,14 @@ static int parse_trailer(struct strbuf *tok, struct strbuf *val,
/* Lookup if the token matches something in the config */
tok_len = token_len_without_separator(tok->buf, tok->len);
- *conf = &default_conf_info;
+ if (conf)
+ *conf = &default_conf_info;
list_for_each(pos, &conf_head) {
- item = list_entry(pos, struct trailer_item, list);
+ item = list_entry(pos, struct arg_item, list);
if (token_matches_item(tok->buf, item, tok_len)) {
char *tok_buf = strbuf_detach(tok, NULL);
- *conf = &item->conf;
+ if (conf)
+ *conf = &item->conf;
strbuf_addstr(tok, token_from_item(item, tok_buf));
free(tok_buf);
break;
@@ -562,43 +591,51 @@ static int parse_trailer(struct strbuf *tok, struct strbuf *val,
return 0;
}
-static void add_trailer_item(struct list_head *head, char *tok, char *val,
- const struct conf_info *conf)
+static void 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;
- duplicate_conf(&new->conf, conf);
list_add_tail(&new->list, head);
}
+static void add_arg_item(struct list_head *arg_head, char *tok, char *val,
+ const struct conf_info *conf)
+{
+ struct arg_item *new = xcalloc(sizeof(*new), 1);
+ new->token = tok;
+ new->value = val;
+ duplicate_conf(&new->conf, conf);
+ list_add_tail(&new->list, arg_head);
+}
+
static void process_command_line_args(struct list_head *arg_head,
struct string_list *trailers)
{
struct string_list_item *tr;
- struct trailer_item *item;
+ struct arg_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 */
+ /* Add an arg item for each configured trailer with a command */
list_for_each(pos, &conf_head) {
- item = list_entry(pos, struct trailer_item, list);
+ item = list_entry(pos, struct arg_item, list);
if (item->conf.command)
- add_trailer_item(arg_head,
- xstrdup(token_from_item(item, NULL)),
- xstrdup(""),
- &item->conf);
+ add_arg_item(arg_head,
+ xstrdup(token_from_item(item, NULL)),
+ xstrdup(""),
+ &item->conf);
}
- /* Add a trailer item for each trailer on the command line */
+ /* Add an arg item for each trailer on the command line */
for_each_string_list_item(tr, trailers) {
if (!parse_trailer(&tok, &val, &conf, tr->string))
- add_trailer_item(arg_head,
- strbuf_detach(&tok, NULL),
- strbuf_detach(&val, NULL),
- conf);
+ add_arg_item(arg_head,
+ strbuf_detach(&tok, NULL),
+ strbuf_detach(&val, NULL),
+ conf);
}
}
@@ -721,7 +758,6 @@ 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;
- const struct conf_info *conf;
/* Get the line count */
while (lines[count])
@@ -740,11 +776,10 @@ 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 &&
- !parse_trailer(&tok, &val, &conf, lines[i]->buf))
+ !parse_trailer(&tok, &val, NULL, lines[i]->buf))
add_trailer_item(head,
strbuf_detach(&tok, NULL),
- strbuf_detach(&val, NULL),
- conf);
+ strbuf_detach(&val, NULL));
}
return trailer_end;
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v3 6/6] trailer: support values folded to multiple lines
From: Jonathan Tan @ 2016-10-14 17:38 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, gitster, jnareb
In-Reply-To: <cover.1476466609.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 | 139 +++++++++++++++++++++++++++++++
trailer.c | 22 +++--
3 files changed, 160 insertions(+), 8 deletions(-)
diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt
index c480da6..cfec636 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 before and 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 7f5cd2a..31db699 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -157,6 +157,145 @@ test_expect_success 'with non-trailer lines only' '
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 d6dfc7a..ef276e6 100644
--- a/trailer.c
+++ b/trailer.c
@@ -248,7 +248,7 @@ static void apply_item_command(struct trailer_item *in_tok, struct arg_item *arg
if (arg_tok->value && arg_tok->value[0]) {
arg = arg_tok->value;
} else {
- if (in_tok && in_tok->value)
+ if (in_tok)
arg = xstrdup(in_tok->value);
else
arg = xstrdup("");
@@ -622,12 +622,14 @@ static int parse_trailer(struct strbuf *tok, struct strbuf *val,
return 0;
}
-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,
@@ -781,6 +783,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])
@@ -798,16 +801,25 @@ static int process_input_file(FILE *outfile,
/* Parse trailer lines */
for (i = trailer_start; i < trailer_end; i++) {
+ 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;
+ }
if (!parse_trailer(&tok, &val, NULL, lines[i]->buf, 0))
- 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
* [PATCH v3 5/6] trailer: allow non-trailers in trailer block
From: Jonathan Tan @ 2016-10-14 17:38 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, gitster, jnareb
In-Reply-To: <cover.1476466609.git.jonathantanmy@google.com>
Currently, interpret-trailers requires all lines of a trailer block to
be trailers (or comments) - if not it would not identify that block as a
trailer block, and thus create its own trailer block, inserting a blank
line. For example:
echo -e "\na: b\nnot trailer" |
git interpret-trailers --trailer "c: d"
would result in:
a: b
not trailer
c: d
Relax the definition of a trailer block to only require 1 trailer, so
that trailers can be directly added to such blocks, resulting in:
a: b
not trailer
c: d
This allows arbitrary lines to be included in trailer blocks, like those
in [1], and still allow interpret-trailers to be used.
This change also makes comments in the trailer block be treated as any
other non-trailer line, preserving them in the output of
interpret-trailers.
[1]
https://kernel.googlesource.com/pub/scm/linux/kernel/git/stable/linux-stable/+/e7d316a02f683864a12389f8808570e37fb90aa3
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
Documentation/git-interpret-trailers.txt | 3 +-
t/t7513-interpret-trailers.sh | 35 +++++++++++++++
trailer.c | 77 ++++++++++++++++++++++----------
3 files changed, 90 insertions(+), 25 deletions(-)
diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt
index 93d1db6..c480da6 100644
--- a/Documentation/git-interpret-trailers.txt
+++ b/Documentation/git-interpret-trailers.txt
@@ -48,7 +48,8 @@ with only spaces at the end of the commit message part, one blank line
will be added before the new trailer.
Existing trailers are extracted from the input message by looking for
-a group of one or more lines that contain a colon (by default), where
+a group of one or more lines in which at least one line contains a
+colon (by default), where
the group is preceded by one or more empty (or whitespace-only) lines.
The group must either be at the end of the message or be the last
non-whitespace lines before a line that starts with '---'. Such three
diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index aee785c..7f5cd2a 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -126,6 +126,37 @@ test_expect_success 'with multiline title in the message' '
test_cmp expected actual
'
+test_expect_success 'with non-trailer lines mixed with trailer lines' '
+ cat >patch <<-\EOF &&
+
+ this: is a trailer
+ this is not a trailer
+ EOF
+ cat >expected <<-\EOF &&
+
+ this: is a trailer
+ this is not a trailer
+ token: value
+ EOF
+ git interpret-trailers --trailer "token: value" patch >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'with non-trailer lines only' '
+ cat >patch <<-\EOF &&
+
+ this is not a trailer
+ EOF
+ cat >expected <<-\EOF &&
+
+ this is not a trailer
+
+ token: value
+ EOF
+ git interpret-trailers --trailer "token: value" patch >actual &&
+ test_cmp expected actual
+'
+
test_expect_success 'with config setup' '
git config trailer.ack.key "Acked-by: " &&
cat >expected <<-\EOF &&
@@ -257,6 +288,8 @@ test_expect_success 'with message that has comments' '
cat >>expected <<-\EOF &&
# comment
+ # other comment
+ # yet another comment
Reviewed-by: Johan
Cc: Peff
# last comment
@@ -286,6 +319,8 @@ test_expect_success 'with message that has an old style conflict block' '
cat >>expected <<-\EOF &&
# comment
+ # other comment
+ # yet another comment
Reviewed-by: Johan
Cc: Peff
# last comment
diff --git a/trailer.c b/trailer.c
index a9ed3f8..d6dfc7a 100644
--- a/trailer.c
+++ b/trailer.c
@@ -27,6 +27,10 @@ static struct conf_info default_conf_info;
struct trailer_item {
struct list_head list;
+ /*
+ * If this is not a trailer line, the line is stored in value
+ * (excluding the terminating newline) and token is NULL.
+ */
char *token;
char *value;
};
@@ -70,9 +74,14 @@ static size_t token_len_without_separator(const char *token, size_t len)
static int same_token(struct trailer_item *a, struct arg_item *b)
{
- size_t a_len = token_len_without_separator(a->token, strlen(a->token));
- size_t b_len = token_len_without_separator(b->token, strlen(b->token));
- size_t min_len = (a_len > b_len) ? b_len : a_len;
+ size_t a_len, b_len, min_len;
+
+ if (!a->token)
+ return 0;
+
+ a_len = token_len_without_separator(a->token, strlen(a->token));
+ b_len = token_len_without_separator(b->token, strlen(b->token));
+ min_len = (a_len > b_len) ? b_len : a_len;
return !strncasecmp(a->token, b->token, min_len);
}
@@ -130,7 +139,14 @@ static char last_non_space_char(const char *s)
static void print_tok_val(FILE *outfile, const char *tok, const char *val)
{
- char c = last_non_space_char(tok);
+ char c;
+
+ if (!tok) {
+ fprintf(outfile, "%s\n", val);
+ return;
+ }
+
+ c = last_non_space_char(tok);
if (!c)
return;
if (strchr(separators, c))
@@ -543,8 +559,16 @@ static int token_matches_item(const char *tok, struct arg_item *item, int tok_le
return item->conf.key ? !strncasecmp(tok, item->conf.key, tok_len) : 0;
}
+/*
+ * Parse the given trailer into token and value parts.
+ *
+ * If the given trailer does not have a separator (and thus cannot be separated
+ * into token and value parts), it is treated as a token (if parse_as_arg) or
+ * as a non-trailer line (if not parse_as_arg).
+ */
static int parse_trailer(struct strbuf *tok, struct strbuf *val,
- const struct conf_info **conf, const char *trailer)
+ const struct conf_info **conf, const char *trailer,
+ int parse_as_arg)
{
size_t len;
struct strbuf seps = STRBUF_INIT;
@@ -557,11 +581,18 @@ static int parse_trailer(struct strbuf *tok, struct strbuf *val,
len = strcspn(trailer, seps.buf);
strbuf_release(&seps);
if (len == 0) {
- int l = strlen(trailer);
+ int l;
+ if (!parse_as_arg)
+ return -1;
+
+ l = strlen(trailer);
while (l > 0 && isspace(trailer[l - 1]))
l--;
return error(_("empty trailer token in trailer '%.*s'"), l, trailer);
}
+ if (!parse_as_arg && len == strlen(trailer))
+ return -1;
+
if (len < strlen(trailer)) {
strbuf_add(tok, trailer, len);
strbuf_trim(tok);
@@ -631,7 +662,7 @@ static void process_command_line_args(struct list_head *arg_head,
/* Add an arg item for each trailer on the command line */
for_each_string_list_item(tr, trailers) {
- if (!parse_trailer(&tok, &val, &conf, tr->string))
+ if (!parse_trailer(&tok, &val, &conf, tr->string, 1))
add_arg_item(arg_head,
strbuf_detach(&tok, NULL),
strbuf_detach(&val, NULL),
@@ -683,7 +714,7 @@ static int find_patch_start(struct strbuf **lines, int count)
*/
static int find_trailer_start(struct strbuf **lines, int count)
{
- int start, end_of_title, only_spaces = 1;
+ int start, end_of_title, only_spaces = 1, trailer_found = 0;
/* The first paragraph is the title and cannot be trailers */
for (start = 0; start < count; start++) {
@@ -699,22 +730,17 @@ static int find_trailer_start(struct strbuf **lines, int count)
* for a line with only spaces before lines with one separator.
*/
for (start = count - 1; start >= end_of_title; start--) {
- if (lines[start]->buf[0] == comment_line_char)
- continue;
if (contains_only_spaces(lines[start]->buf)) {
if (only_spaces)
continue;
- return start + 1;
+ return trailer_found ? start + 1 : count;
}
- if (strcspn(lines[start]->buf, separators) < lines[start]->len) {
- if (only_spaces)
- only_spaces = 0;
- continue;
- }
- return count;
+ only_spaces = 0;
+ if (strcspn(lines[start]->buf, separators) < lines[start]->len)
+ trailer_found = 1;
}
- return only_spaces ? count : 0;
+ return count;
}
/* Get the index of the end of the trailers */
@@ -735,11 +761,8 @@ static int find_trailer_end(struct strbuf **lines, int patch_start)
static int has_blank_line_before(struct strbuf **lines, int start)
{
- for (;start >= 0; start--) {
- if (lines[start]->buf[0] == comment_line_char)
- continue;
+ if (start >= 0)
return contains_only_spaces(lines[start]->buf);
- }
return 0;
}
@@ -775,11 +798,17 @@ 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 &&
- !parse_trailer(&tok, &val, NULL, lines[i]->buf))
+ if (!parse_trailer(&tok, &val, NULL, lines[i]->buf, 0))
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));
+ }
}
return trailer_end;
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v3 3/6] trailer: streamline trailer item create and add
From: Jonathan Tan @ 2016-10-14 17:38 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, gitster, jnareb
In-Reply-To: <cover.1476466609.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 0afa240..54cc930 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 v3 2/6] trailer: use list.h for doubly-linked list
From: Jonathan Tan @ 2016-10-14 17:37 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, gitster, jnareb
In-Reply-To: <cover.1476466609.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>
---
trailer.c | 258 ++++++++++++++++++++++----------------------------------------
1 file changed, 91 insertions(+), 167 deletions(-)
diff --git a/trailer.c b/trailer.c
index 1f191b2..0afa240 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;
+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 v3 1/6] trailer: improve const correctness
From: Jonathan Tan @ 2016-10-14 17:37 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, gitster, jnareb
In-Reply-To: <cover.1476466609.git.jonathantanmy@google.com>
Change "const char *" to "char *" in struct trailer_item and in the
return value of apply_command (since those strings are owned strings).
Change "struct conf_info *" to "const struct conf_info *" (since that
struct is not modified).
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
trailer.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/trailer.c b/trailer.c
index c6ea9ac..1f191b2 100644
--- a/trailer.c
+++ b/trailer.c
@@ -27,8 +27,8 @@ static struct conf_info default_conf_info;
struct trailer_item {
struct trailer_item *previous;
struct trailer_item *next;
- const char *token;
- const char *value;
+ char *token;
+ char *value;
struct conf_info conf;
};
@@ -95,8 +95,8 @@ static void free_trailer_item(struct trailer_item *item)
free(item->conf.name);
free(item->conf.key);
free(item->conf.command);
- free((char *)item->token);
- free((char *)item->value);
+ free(item->token);
+ free(item->value);
free(item);
}
@@ -215,13 +215,13 @@ static struct trailer_item *remove_first(struct trailer_item **first)
return item;
}
-static const char *apply_command(const char *command, const char *arg)
+static char *apply_command(const char *command, const char *arg)
{
struct strbuf cmd = STRBUF_INIT;
struct strbuf buf = STRBUF_INIT;
struct child_process cp = CHILD_PROCESS_INIT;
const char *argv[] = {NULL, NULL};
- const char *result;
+ char *result;
strbuf_addstr(&cmd, command);
if (arg)
@@ -425,7 +425,7 @@ static int set_if_missing(struct conf_info *item, const char *value)
return 0;
}
-static void duplicate_conf(struct conf_info *dst, struct conf_info *src)
+static void duplicate_conf(struct conf_info *dst, const struct conf_info *src)
{
*dst = *src;
if (src->name)
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v3 0/6] allow non-trailers and multiple-line trailers
From: Jonathan Tan @ 2016-10-14 17:37 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, gitster, jnareb
In-Reply-To: <cover.1476232683.git.jonathantanmy@google.com>
Ah, I knew I forgot something. These are exactly the same as v2, except
that these are signed off.
Jonathan Tan (6):
trailer: improve const correctness
trailer: use list.h for doubly-linked list
trailer: streamline trailer item create and add
trailer: make args have their own struct
trailer: allow non-trailers in trailer block
trailer: support values folded to multiple lines
Documentation/git-interpret-trailers.txt | 10 +-
t/t7513-interpret-trailers.sh | 174 ++++++++++
trailer.c | 538 +++++++++++++++----------------
3 files changed, 444 insertions(+), 278 deletions(-)
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply
* Re: [PATCH v2 1/3] gitweb: Fix a typo in a comment
From: Jakub Narębski @ 2016-10-14 17:34 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason, git; +Cc: Junio C Hamano
In-Reply-To: <20161006091135.29590-2-avarab@gmail.com>
W dniu 06.10.2016 o 11:11, Ævar Arnfjörð Bjarmason napisał:
> Change a typo'd MIME type in a comment. The Content-Type is
> application/xhtml+xml, not application/xhtm+xml.
>
> Fixes up code originally added in 53c4031 ("gitweb: Strip
> non-printable characters from syntax highlighter output", 2011-09-16).
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Good. Thanks for taking care of this.
For what is worth for such a trivial patch:
Acked-by: Jakub Narębski <jnareb@gmail.com>
> ---
> gitweb/gitweb.perl | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 44094f4..cba7405 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1616,7 +1616,7 @@ sub esc_path {
> return $str;
> }
>
> -# Sanitize for use in XHTML + application/xml+xhtm (valid XML 1.0)
> +# Sanitize for use in XHTML + application/xml+xhtml (valid XML 1.0)
> sub sanitize {
> my $str = shift;
>
>
^ permalink raw reply
* Re: Formatting problem send_mail in version 2.10.0
From: Junio C Hamano @ 2016-10-14 17:04 UTC (permalink / raw)
To: Matthieu Moy
Cc: Larry Finger, Jeff King, Mathieu Lienard--Mayor, Remi Lespinet,
git
In-Reply-To: <vpq1szkolud.fsf@anie.imag.fr>
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> We clearly can't guess, but we can be consistent with Mail::Address, so
> that git's behavior depends less on its availability.
>
> Patch follows doing that.
Thanks. I love that somebody counters with a much better way to
solve it whenever I suggest an outrageous non-solution to a problem
around here ;-)
^ permalink raw reply
* Re: Uninitialized submodules as symlinks
From: Junio C Hamano @ 2016-10-14 16:48 UTC (permalink / raw)
To: Kevin Daudt; +Cc: Heiko Voigt, David Turner, git@vger.kernel.org
In-Reply-To: <20161013193558.GC1698@ikke.info>
Kevin Daudt <me@ikke.info> writes:
> On Thu, Oct 13, 2016 at 06:10:17PM +0200, Heiko Voigt wrote:
>> On Fri, Oct 07, 2016 at 06:17:05PM +0000, David Turner wrote:
>> > Presently, uninitialized submodules are materialized in the working
>> > tree as empty directories. We would like to consider having them be
>> > symlinks. Specifically, we'd like them to be symlinks into a FUSE
>> > filesystem which retrieves files on demand.
>>
>> How about portability? This feature would only work on Unix like
>> operating systems. You have to be careful to not break Windows since
>> they do not have symlinks.
>
> NTFS does have symlinks, but you need admin right to create them though
> (unless you change the policy).
That sounds like saying "It has, but it practically is not usable by
Git as a mechanism to achieve this goal" to me.
^ permalink raw reply
* Re: [PATCH v3 13/25] sequencer: prepare for rebase -i's commit functionality
From: Junio C Hamano @ 2016-10-14 16:41 UTC (permalink / raw)
To: Johannes Schindelin
Cc: git, Jakub Narębski, Johannes Sixt, Michael Haggerty
In-Reply-To: <alpine.DEB.2.20.1610131242100.197091@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> I think my puzzlement comes from here. What makes it OK for "am" to
>> expect the contents of author-script file to be quoted but it is not
>> OK to expect the same here? What makes it not quoted for _this_
>> reader, in other words?
>
> The `git am` command is inherently *not* interactive, while the
> interactive rebase, well, is.
>
> As such, we must assume that enterprisey users did come up with scripts
> that edit, or create, author-script files, and exploited the fact that the
> interactive rebase previously sourced them.
It's like saying "While the program is waiting for user interaction,
the user can attach a debugger to tweak the contents of this string
variable. Hence we must not assume that the string is always NUL
terminated to protect ourselves."
A correct response to such a user is to tell them not to do that,
and do so at two levels.
The first level may be "it may be physically possible to tweak the
string via debugger but don't do that in such a way that breaks the
invariants our program relies on, or you are on your own", but more
important is the response at the second level. Why is the user
futzing with that string in the first place with the debugger? In
the context of author-script, the question is "Why is the user
futzing with the author-script file"? To attribute the resulting
commit to a different author, of course. But we need to step back
and think why the user needs to resort to editing that file to
achieve that.
If that is a common thing users would want to do, then we should
offer an official way to do so, and it should not be "you can futz
with author-script file with your editor". Something like "have
'exec git commit --amend' in the todo file" may be more appropriate
and if it is important enough, the sequencer command language may
want to learn an extra verb to update the author.
Besides, the opportunity easiest for the user to futz with the
contents of author-script file (or "attach the debugger while we
wait for user interaction") arises when "rebase -i" or "am" stops
waiting for conflict resolution. Even when you run "am" without its
"-i" option, it is equally susceptible to the "user futzing with the
file", which means your "am is not interactive but 'rebase -i' is"
is irrelevant to the issue. Oh, also, did I say "am" has "-i"
option, which is a short-hand for "--interactive"?
What disturbs me the most is that I know you know the system well
enough to realize how bogus your argument to claim that "rebase -i"
and "am" are different was and to come up with what I wrote above
yourself. Which means that I need to conclude that you have some
other reasons why you want to keep this parser different, but I
still do not know what they are.
I guess it entirely is possible that one of the reasons is because
some later patches in the larger "rebase-i to sequencer" series
writes author-script file in a syntax that cannot be read by the
recently refactored code "am" uses to read the author-script file,
and reusing the existing code may end up breaking the endgame
"rebase -i" you have.
As I do not feel like arguing with you on this any longer, and as I
certainly do not want to be blamed for breaking your "rebase -i", I
do not insist the code to be refactored to share with the existing
codepath. But still I do not see the need to keep them separate (as
I already said in the previous message, I am OK if the one used in
"am" is updated to match).
^ permalink raw reply
* [PATCH v15 27/27] bisect--helper: remove the dequote in bisect_start()
From: Pranit Bauva @ 2016-10-14 14:14 UTC (permalink / raw)
To: git
In-Reply-To: <01020157c38b19e0-81123fa5-5d9d-4f64-8f1b-ff336e83ebe4-000000@eu-west-1.amazonses.com>
Dequoting the arguments was introduced in 25b48b5c to port the function
`bisect_next()` but after the `bisect_replay()` porting, the dequoting
is carried out itself when it passes the arguments to `bisect_start()`
in a simpler way thus dequoting again isn't required. So remove the
extra "deqouting" code introduced by the commit 25b48b5c.
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
---
builtin/bisect--helper.c | 26 ++++++--------------------
1 file changed, 6 insertions(+), 20 deletions(-)
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index d6c2efc..8cd6527 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -561,24 +561,16 @@ static int bisect_start(struct bisect_terms *terms, int no_checkout,
no_checkout = 1;
for (i = 0; i < argc; i++) {
- const char *arg;
- if (starts_with(argv[i], "'"))
- arg = sq_dequote(xstrdup(argv[i]));
- else
- arg = argv[i];
- if (!strcmp(arg, "--")) {
+ if (!strcmp(argv[i], "--")) {
has_double_dash = 1;
break;
}
}
for (i = 0; i < argc; i++) {
- const char *arg, *commit_id;
- if (starts_with(argv[i], "'"))
- arg = sq_dequote(xstrdup(argv[i]));
- else
- arg = argv[i];
- commit_id = xstrfmt("%s^{commit}", arg);
+ const char *commit_id;
+ const char *arg = argv[i];
+ commit_id = xstrfmt("%s^{commit}", argv[i]);
if (!strcmp(argv[i], "--")) {
has_double_dash = 1;
break;
@@ -586,11 +578,8 @@ static int bisect_start(struct bisect_terms *terms, int no_checkout,
no_checkout = 1;
} else if (!strcmp(arg, "--term-good") ||
!strcmp(arg, "--term-old")) {
- if (starts_with(argv[++i], "'"))
- terms->term_good = sq_dequote(xstrdup(argv[i]));
- else
- terms->term_good = xstrdup(argv[i]);
must_write_terms = 1;
+ terms->term_good = xstrdup(argv[++i]);
} else if (skip_prefix(arg, "--term-good=", &arg)) {
must_write_terms = 1;
terms->term_good = arg;
@@ -599,10 +588,7 @@ static int bisect_start(struct bisect_terms *terms, int no_checkout,
terms->term_good = arg;
} else if (!strcmp(arg, "--term-bad") ||
!strcmp(arg, "--term-new")) {
- if (starts_with(argv[++i], "'"))
- terms->term_bad = sq_dequote(xstrdup(argv[i]));
- else
- terms->term_bad = xstrdup(argv[i]);
+ terms->term_bad = xstrdup(argv[++i]);
must_write_terms = 1;
} else if (skip_prefix(arg, "--term-bad=", &arg)) {
must_write_terms = 1;
--
https://github.com/git/git/pull/287
^ permalink raw reply related
* [PATCH v15 11/27] bisect--helper: `bisect_next_check` & bisect_voc shell function in C
From: Pranit Bauva @ 2016-10-14 14:14 UTC (permalink / raw)
To: git
In-Reply-To: <01020157c38b19e0-81123fa5-5d9d-4f64-8f1b-ff336e83ebe4-000000@eu-west-1.amazonses.com>
Reimplement `bisect_next_check` shell function in C and add
`bisect-next-check` subcommand to `git bisect--helper` to call it from
git-bisect.sh .
Also reimplement `bisect_voc` shell function in C and call it from
`bisect_next_check` implementation in C.
Using `--bisect-next-check` is a temporary measure to port shell
function to C so as to use the existing test suite. As more functions
are ported, this subcommand will be retired but its implementation will
be called by some other methods.
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
---
builtin/bisect--helper.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++-
git-bisect.sh | 60 ++------------------------
2 files changed, 113 insertions(+), 57 deletions(-)
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index c6c11e3..317d671 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -6,6 +6,7 @@
#include "dir.h"
#include "argv-array.h"
#include "run-command.h"
+#include "prompt.h"
static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
@@ -21,6 +22,7 @@ static const char * const git_bisect_helper_usage[] = {
N_("git bisect--helper --bisect-reset [<commit>]"),
N_("git bisect--helper --bisect-write <state> <revision> <TERM_GOOD> <TERM_BAD> [<nolog>]"),
N_("git bisect--helper --bisect-check-and-set-terms <command> <TERM_GOOD> <TERM_BAD>"),
+ N_("git bisect--helper --bisect-next-check [<term>] <TERM_GOOD> <TERM_BAD"),
NULL
};
@@ -245,6 +247,102 @@ static int check_and_set_terms(struct bisect_terms *terms, const char *cmd)
return 0;
}
+static int mark_good(const char *refname, const struct object_id *oid,
+ int flag, void *cb_data)
+{
+ int *m_good = (int *)cb_data;
+ *m_good = 0;
+ return 1;
+}
+
+static char *bisect_voc(char *revision_type)
+{
+ if (!strcmp(revision_type, "bad"))
+ return "bad|new";
+ if (!strcmp(revision_type, "good"))
+ return "good|old";
+
+ return NULL;
+}
+
+static int bisect_next_check(const struct bisect_terms *terms,
+ const char *current_term)
+{
+ int missing_good = 1, missing_bad = 1, retval = 0;
+ char *bad_ref = xstrfmt("refs/bisect/%s", terms->term_bad);
+ char *good_glob = xstrfmt("%s-*", terms->term_good);
+ char *bad_syn, *good_syn;
+
+ if (ref_exists(bad_ref))
+ missing_bad = 0;
+
+ for_each_glob_ref_in(mark_good, good_glob, "refs/bisect/",
+ (void *) &missing_good);
+
+ if (!missing_good && !missing_bad)
+ goto finish;
+
+ if (!current_term) {
+ retval = -1;
+ goto finish;
+ }
+
+ if (missing_good && !missing_bad && current_term &&
+ !strcmp(current_term, terms->term_good)) {
+ char *yesno;
+ /*
+ * have bad (or new) but not good (or old). We could bisect
+ * although this is less optimum.
+ */
+ fprintf(stderr, _("Warning: bisecting only with a %s commit\n"),
+ terms->term_bad);
+ if (!isatty(0))
+ goto finish;
+ /*
+ * TRANSLATORS: Make sure to include [Y] and [n] in your
+ * translation. The program will only accept English input
+ * at this point.
+ */
+ yesno = git_prompt(_("Are you sure [Y/n]? "), PROMPT_ECHO);
+ if (starts_with(yesno, "N") || starts_with(yesno, "n")) {
+ retval = -1;
+ goto finish;
+ }
+
+ goto finish;
+ }
+ bad_syn = xstrdup(bisect_voc("bad"));
+ good_syn = xstrdup(bisect_voc("good"));
+ if (!is_empty_or_missing_file(git_path_bisect_start())) {
+ error(_("You need to give me at least one %s and "
+ "%s revision. You can use \"git bisect %s\" "
+ "and \"git bisect %s\" for that. \n"),
+ bad_syn, good_syn, bad_syn, good_syn);
+ retval = -1;
+ goto finish;
+ }
+ else {
+ error(_("You need to start by \"git bisect start\". You "
+ "then need to give me at least one %s and %s "
+ "revision. You can use \"git bisect %s\" and "
+ "\"git bisect %s\" for that.\n"),
+ good_syn, bad_syn, bad_syn, good_syn);
+ retval = -1;
+ goto finish;
+ }
+ goto finish;
+finish:
+ if (!bad_ref)
+ free(bad_ref);
+ if (!good_glob)
+ free(good_glob);
+ if (!bad_syn)
+ free(bad_syn);
+ if (!good_syn)
+ free(good_syn);
+ return retval;
+}
+
int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
{
enum {
@@ -254,7 +352,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
BISECT_RESET,
CHECK_EXPECTED_REVS,
BISECT_WRITE,
- CHECK_AND_SET_TERMS
+ CHECK_AND_SET_TERMS,
+ BISECT_NEXT_CHECK
} cmdmode = 0;
int no_checkout = 0, res = 0;
struct option options[] = {
@@ -272,6 +371,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
N_("write out the bisection state in BISECT_LOG"), BISECT_WRITE),
OPT_CMDMODE(0, "check-and-set-terms", &cmdmode,
N_("check and set terms in a bisection state"), CHECK_AND_SET_TERMS),
+ OPT_CMDMODE(0, "bisect-next-check", &cmdmode,
+ N_("check whether bad or good terms exist"), BISECT_NEXT_CHECK),
OPT_BOOL(0, "no-checkout", &no_checkout,
N_("update BISECT_HEAD instead of checking out the current commit")),
OPT_END()
@@ -321,6 +422,13 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
terms.term_bad = xstrdup(argv[2]);
res = check_and_set_terms(&terms, argv[0]);
break;
+ case BISECT_NEXT_CHECK:
+ if (argc != 2 && argc != 3)
+ die(_("--bisect-next-check requires 2 or 3 arguments"));
+ terms.term_good = xstrdup(argv[0]);
+ terms.term_bad = xstrdup(argv[1]);
+ res = bisect_next_check(&terms, argc == 3 ? argv[2] : NULL);
+ break;
default:
die("BUG: unknown subcommand '%d'", cmdmode);
}
diff --git a/git-bisect.sh b/git-bisect.sh
index bdf2227..fe6c9d0 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -271,59 +271,14 @@ bisect_state() {
bisect_auto_next
}
-bisect_next_check() {
- missing_good= missing_bad=
- git show-ref -q --verify refs/bisect/$TERM_BAD || missing_bad=t
- test -n "$(git for-each-ref "refs/bisect/$TERM_GOOD-*")" || missing_good=t
-
- case "$missing_good,$missing_bad,$1" in
- ,,*)
- : have both $TERM_GOOD and $TERM_BAD - ok
- ;;
- *,)
- # do not have both but not asked to fail - just report.
- false
- ;;
- t,,"$TERM_GOOD")
- # have bad (or new) but not good (or old). we could bisect although
- # this is less optimum.
- eval_gettextln "Warning: bisecting only with a \$TERM_BAD commit." >&2
- if test -t 0
- then
- # TRANSLATORS: Make sure to include [Y] and [n] in your
- # translation. The program will only accept English input
- # at this point.
- gettext "Are you sure [Y/n]? " >&2
- read yesno
- case "$yesno" in [Nn]*) exit 1 ;; esac
- fi
- : bisect without $TERM_GOOD...
- ;;
- *)
- bad_syn=$(bisect_voc bad)
- good_syn=$(bisect_voc good)
- if test -s "$GIT_DIR/BISECT_START"
- then
-
- eval_gettextln "You need to give me at least one \$bad_syn and one \$good_syn revision.
-(You can use \"git bisect \$bad_syn\" and \"git bisect \$good_syn\" for that.)" >&2
- else
- eval_gettextln "You need to start by \"git bisect start\".
-You then need to give me at least one \$good_syn and one \$bad_syn revision.
-(You can use \"git bisect \$bad_syn\" and \"git bisect \$good_syn\" for that.)" >&2
- fi
- exit 1 ;;
- esac
-}
-
bisect_auto_next() {
- bisect_next_check && bisect_next || :
+ git bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD && bisect_next || :
}
bisect_next() {
case "$#" in 0) ;; *) usage ;; esac
bisect_autostart
- bisect_next_check $TERM_GOOD
+ git bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD $TERM_GOOD|| exit
# Perform all bisection computation, display and checkout
git bisect--helper --next-all $(test -f "$GIT_DIR/BISECT_HEAD" && echo --no-checkout)
@@ -355,7 +310,7 @@ bisect_next() {
}
bisect_visualize() {
- bisect_next_check fail
+ git bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit
if test $# = 0
then
@@ -409,7 +364,7 @@ bisect_replay () {
}
bisect_run () {
- bisect_next_check fail
+ git bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit
while true
do
@@ -482,13 +437,6 @@ get_terms () {
fi
}
-bisect_voc () {
- case "$1" in
- bad) echo "bad|new" ;;
- good) echo "good|old" ;;
- esac
-}
-
bisect_terms () {
get_terms
if ! test -s "$GIT_DIR/BISECT_TERMS"
--
https://github.com/git/git/pull/287
^ permalink raw reply related
* Re: What's cooking in git.git (Oct 2016, #03; Tue, 11)
From: Junio C Hamano @ 2016-10-14 16:10 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Stefan Beller, git@vger.kernel.org
In-Reply-To: <alpine.DEB.2.20.1610131439400.197091@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> >> * sb/submodule-ignore-trailing-slash (2016-10-10) 2 commits
>> >> (merged to 'next' on 2016-10-11 at e37425ed17)
>> >> + submodule: ignore trailing slash in relative url
>> >> + submodule: ignore trailing slash on superproject URL
>> >>
>> >> A minor regression fix for "git submodule".
>> >>
>> >> Will merge to 'master'.
>> >
>> > Going by the bug report, this *may* be more than
>> > minor and worth merging down to maint as well, eventually.
>>
>> The topic was forked at a reasonably old commit so that it can be
>> merged as far down to maint-2.9 if we wanted to. Which means the
>> regression was fairly old and fix is not all that urgent as well.
>
> And if you merge it to `master` and `maint`,...
I'll mark it as "wait for follow-up fix" in whats-cooking.txt (on
'todo' branch) to remind myself not to merge it yet.
Thanks for reminding.
As I was mostly offline yesterday, it will take a while until I
clear my backlog on the list.
^ permalink raw reply
* Re: Why does git checkout -b touch the index?
From: Junio C Hamano @ 2016-10-14 16:07 UTC (permalink / raw)
To: David Turner; +Cc: git
In-Reply-To: <1476455756.32360.2.camel@frank>
David Turner <novalis@novalis.org> writes:
> If I do "git checkout -b fleem", with no additional flags, why does it
> need to rewrite the index? Or even read the index?
The "reading" part can be explained easily.
It needs to show the list of dirty paths, which involves reading the
index, refreshing them in-core to cull the timestamp-only changes
out of the report.
Once in-core index is refreshed, it is a waste not to write them out
if it can, so it is not surprising if it writes, too.
The real reason is a lot more involved. "git checkout -b new" is
different from "git update-ref refs/heads/new HEAD" (i.e. create a
new branch at the same commit) followed by "git symbolic-ref HEAD
refs/heads/new" (i.e. point the HEAD at it). If it were, I fully
agree with you that it shouldn't need to touch either index or
working tree at all.
It is "git branch new" followed by "git checkout new", and the
latter step does the full two-tree "read-tree" between the same two
commits. I personally very much prefer "Switching between the same
two commits? That should be a no-op by definition.", but we cannot
blindly that naive optimization, as it would break some use cases
(which I personally do not care too much about ;-).
For example, extra working tree files outside the "sparse" area are
removed when sparse-checkout is in use while switching from the
current branch to the new and identical branch. I do not personally
agree with the behaviour, but that has been what the users accept;
there may be other funnies like that.
^ permalink raw reply
* Re: [PATCHv3] attr: convert to new threadsafe API
From: Junio C Hamano @ 2016-10-14 15:37 UTC (permalink / raw)
To: Stefan Beller; +Cc: bmwill, git, j6t, jacob.keller
In-Reply-To: <xmqqfuo116t0.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> *1* Would we need a wrapping struct around the array of results?
By the way, I do see a merit on the "check" side (tl;dr: but I do
not think "result" needs it, hence I do not see the need for the
"ugly" variants).
Take "archive" for example. For each path, it wants to see the
attribute "export-ignore" to decide if it is to be omitted. In
addition, the usual set of attributes used to smudge blobs into the
working tree representation are inspected by the convert.c API as
part of its implementation of convert_to_working_tree(). This
program has at least two sets of <"check", "result"> that are used
by two git_check_attr() callsites that are unaware of each other.
One of the optimizations we discussed is to trim down the attr-stack
(which caches the attributes read from .gitattributes files that are
in effect for the "last" directory that has the path for which
attrbiutes are queried for) by reading/keeping only the entries that
affect the attributes the caller is interested in. But when there
are multiple callsites that are interested in different sets of
attributes, we obviously cannot do such an optimization without
taking too much cache-invalidation hit. Because these callsites are
not unaware of each other, I do not think we can say "keep the
entries that affects the union of all active callsites" very easily,
even if it were possible.
But we could tie this cache to "check", which keeps a constant
subset of attributes that the caller is interested in (i.e. each
callsite would keep its own cache that is useful for its query).
While we are single-threaded, "struct git_attr_check" being a
wrapping struct around the array of "what attributes are of
interest?" is a good place to add that per-check attr-stack cache.
When we go multi-threaded, the attr-stack cache must become
per-thread, and needs to be moved to per-thread storage, and such a
per-thread storage would have multiple attr-stack, one per "check"
instance (i.e. looking up the attr-stack may have to say "who/what
thread am I?" to first go to the thread-local storage for the
current thread, where a table of pointers to attr-stacks is kept and
from there, index into that table to find the attr-stack that
corresponds to the particular "check"). We could use the address of
"check" as the key into this table, but "struct git_attr_check" that
wraps the array gives us another option to allocate a small
consecutive integer every time initl() creates a new "check" and use
it as the index into that attr-stack table, as that integer index
can be in the struct that wraps the array of wanted attributes.
Note. none of the above is a suggestion to do the attr
caching the way exactly described. The above is primarily
to illustrate how a wrapping struct may give us future
flexibility without affecting a single line of code in the
user of API.
It may turn out that we do not need to have anything other than the
array of wanted attributes in the "check" struct, but unlike
"result", "check" is shared across threads, and do not have to live
directly on the stack, so we can prepare for flexibility.
I do not foresee a similar need for wrapping struct for "result",
and given that we do want to keep the option of having them directly
on the stack, I am inclined to say we shouldn't introduce one.
If we were still to do the wrapping for result, I would say that
basing it around the FLEX_ARRAY idiom, i.e.
> struct git_attr_result {
> int num_slots;
> const char *value[FLEX_ARRAY];
> };
is a horrible idea. It would be less horrible if it were
struct git_attr_result {
int num_slots;
const char **value;
};
then make the API user write via a convenience macro something like
this
const char *result_values[NUM_ATTRS_OF_INTEREST];
struct git_attr_result result = {
ARRAY_SIZE(result_values), &result_values
};
instead. That way, at least the side that implements git_check_attr()
would not have to be type-unsafe like the example of ugliness in the
message I am following-up on.
^ permalink raw reply
* Re: [PATCH v15 01/27] bisect--helper: use OPT_CMDMODE instead of OPT_BOOL
From: Pranit Bauva @ 2016-10-14 15:12 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Christian Couder, Christian Couder,
Lars Schneider
In-Reply-To: <01020157c38b19e0-81123fa5-5d9d-4f64-8f1b-ff336e83ebe4-000000@eu-west-1.amazonses.com>
Hey everyone,
It took me some time to get to the next version as I was a bit
preoccupied with my assignments and exams. The diff between the v14[1]
and v15[2] can be found here[3] because gmail web client will wrap the
lines. Thanks Junio for the reviews in v14. I have tried to solves
every issue raised in the previous round.
The major changes in this series are:
* Use the char * instead of struct strbuf in struct bisect_terms.
* Use goto mechanism for error handling.
* Use get_sha1_committish() instead of just get_oid() because there
is also ^{commit} at the end of the call from shell.
* Use skip_prefix() where ever required.
* Use return error() instead of die() where ever possible.
* Restructure a part in bisect_start() to make it more readable.
* Change the comments in bisect.c to use return instead of exit
because the code does it.
* Introduce a test to check for cleanups in after bad merge base to
make the patch 15/28 more understood.
* Also keeping comments in bisect.c to show at which places there
would be cleanups and where not.
* Restructure bisect_replay() and get_next_word().
[1]: http://public-inbox.org/git/01020156b73fe5b4-5dc768ab-b73b-4a21-ab92-018e2a7aa6f7-000000@eu-west-1.amazonses.com/T/#m7c26060fcf95abbd19f93742d7317eef87b915a1
[2]: http://public-inbox.org/git/01020157c38b19e0-81123fa5-5d9d-4f64-8f1b-ff336e83ebe4-000000@eu-west-1.amazonses.com/T/#u
[3]: http://paste.ubuntu.com/23323581/
Regards,
Pranit Bauva
^ permalink raw reply
* [PATCH v15 25/27] bisect--helper: retire `--bisect-autostart` subcommand
From: Pranit Bauva @ 2016-10-14 14:14 UTC (permalink / raw)
To: git
In-Reply-To: <01020157c38b19e0-81123fa5-5d9d-4f64-8f1b-ff336e83ebe4-000000@eu-west-1.amazonses.com>
The `--bisect-autostart` subcommand is no longer used in the shell
script and the function `bisect_autostart()` is called from the C
implementation.
Mentored-by: Lars Schneider <larsxschneider@gmail.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
---
builtin/bisect--helper.c | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 7f676e2..8653fde 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -28,7 +28,6 @@ static const char * const git_bisect_helper_usage[] = {
"[--no-checkout] [<bad> [<good>...]] [--] [<paths>...]"),
N_("git bisect--helper --bisect-next"),
N_("git bisect--helper --bisect-auto-next"),
- N_("git bisect--helper --bisect-autostart"),
N_("git bisect--helper --bisect-state (bad|new) [<rev>]"),
N_("git bisect--helper --bisect-state (good|old) [<rev>...]"),
N_("git bisect--helper --bisect-replay <filename>"),
@@ -995,7 +994,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
BISECT_START,
BISECT_NEXT,
BISECT_AUTO_NEXT,
- BISECT_AUTOSTART,
BISECT_STATE,
BISECT_LOG,
BISECT_REPLAY
@@ -1016,8 +1014,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
N_("find the next bisection commit"), BISECT_NEXT),
OPT_CMDMODE(0, "bisect-auto-next", &cmdmode,
N_("verify the next bisection state then find the next bisection state"), BISECT_AUTO_NEXT),
- OPT_CMDMODE(0, "bisect-autostart", &cmdmode,
- N_("start the bisection if BISECT_START empty or missing"), BISECT_AUTOSTART),
OPT_CMDMODE(0, "bisect-state", &cmdmode,
N_("mark the state of ref (or refs)"), BISECT_STATE),
OPT_CMDMODE(0, "bisect-log", &cmdmode,
@@ -1079,13 +1075,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
get_terms(&terms);
res = bisect_auto_next(&terms, prefix);
break;
- case BISECT_AUTOSTART:
- if (argc)
- die(_("--bisect-autostart requires 0 arguments"));
- terms.term_good = "good";
- terms.term_bad = "bad";
- res = bisect_autostart(&terms);
- break;
case BISECT_STATE:
if (argc == 0)
die(_("--bisect-state requires at least 1 argument"));
--
https://github.com/git/git/pull/287
^ 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