* [PATCH RFC v2 07/25] cat-file: start migrating formatting to ref-filter
2018-02-05 11:27 ` [PATCH RFC v2 01/25] " Olga Telezhnaya
@ 2018-02-05 11:27 ` Olga Telezhnaya
2018-02-05 11:27 ` [PATCH RFC v2 23/25] for-each-ref: tests for new atoms added Olga Telezhnaya
` (23 subsequent siblings)
24 siblings, 0 replies; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-05 11:27 UTC (permalink / raw)
To: git
Move mark_atom_in_object_info() from cat-file to ref-filter and
start using it in verify_ref_format().
It also means that we start reusing verify_ref_format() in cat-file.
Start from simple moving of mark_atom_in_object_info(),
it would be removed later by integrating all needed processes into
ref-filter logic.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
builtin/cat-file.c | 30 +++---------------------------
ref-filter.c | 41 ++++++++++++++++++++++++++++++++++++-----
ref-filter.h | 12 ++++++------
3 files changed, 45 insertions(+), 38 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index edb04a96d9bd3..909412747cbd2 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -182,25 +182,6 @@ static int is_atom(const char *atom, const char *s, int slen)
return alen == slen && !memcmp(atom, s, alen);
}
-static void mark_atom_in_object_info(const char *atom, int len,
- struct expand_data *data)
-{
- if (is_atom("objectname", atom, len))
- ; /* do nothing */
- else if (is_atom("objecttype", atom, len))
- data->info.typep = &data->type;
- else if (is_atom("objectsize", atom, len))
- data->info.sizep = &data->size;
- else if (is_atom("objectsize:disk", atom, len))
- data->info.disk_sizep = &data->disk_size;
- else if (is_atom("rest", atom, len))
- data->split_on_whitespace = 1;
- else if (is_atom("deltabase", atom, len))
- data->info.delta_base_sha1 = data->delta_base_oid.hash;
- else
- die("unknown format element: %.*s", len, atom);
-}
-
static void expand_atom(struct strbuf *sb, const char *atom, int len,
struct expand_data *data)
{
@@ -230,11 +211,7 @@ static size_t expand_format(struct strbuf *sb, const char *start, void *vdata)
if (!end)
die("format element '%s' does not end in ')'", start);
- if (data->mark_query)
- mark_atom_in_object_info(start + 1, end - start - 1, data);
- else
- expand_atom(sb, start + 1, end - start - 1, data);
-
+ expand_atom(sb, start + 1, end - start - 1, data);
return end - start + 1;
}
@@ -418,9 +395,8 @@ static int batch_objects(struct batch_options *opt)
* object.
*/
memset(&data, 0, sizeof(data));
- data.mark_query = 1;
- strbuf_expand(&buf, opt->format.format, expand_format, &data);
- data.mark_query = 0;
+ opt->format.cat_file_data = &data;
+ verify_ref_format(&opt->format);
if (opt->cmdmode)
data.split_on_whitespace = 1;
diff --git a/ref-filter.c b/ref-filter.c
index 5e7ed0f338490..ff87e632f463c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -392,6 +392,31 @@ struct atom_value {
struct used_atom *atom;
};
+static int is_atom(const char *atom, const char *s, int slen)
+{
+ int alen = strlen(atom);
+ return alen == slen && !memcmp(atom, s, alen);
+}
+
+static void mark_atom_in_object_info(const char *atom, int len,
+ struct expand_data *data)
+{
+ if (is_atom("objectname", atom, len))
+ ; /* do nothing */
+ else if (is_atom("objecttype", atom, len))
+ data->info.typep = &data->type;
+ else if (is_atom("objectsize", atom, len))
+ data->info.sizep = &data->size;
+ else if (is_atom("objectsize:disk", atom, len))
+ data->info.disk_sizep = &data->disk_size;
+ else if (is_atom("rest", atom, len))
+ data->split_on_whitespace = 1;
+ else if (is_atom("deltabase", atom, len))
+ data->info.delta_base_sha1 = data->delta_base_oid.hash;
+ else
+ die("unknown format element: %.*s", len, atom);
+}
+
/*
* Used to parse format string and sort specifiers
*/
@@ -709,12 +734,18 @@ int verify_ref_format(struct ref_format *format)
if (!ep)
return error(_("malformed format string %s"), sp);
/* sp points at "%(" and ep points at the closing ")" */
- at = parse_ref_filter_atom(format, valid_atom,
- ARRAY_SIZE(valid_atom), sp + 2, ep);
- cp = ep + 1;
- if (skip_prefix(used_atom[at].name, "color:", &color))
- format->need_color_reset_at_eol = !!strcmp(color, "reset");
+ if (format->cat_file_data)
+ mark_atom_in_object_info(sp + 2, ep - sp - 2,
+ format->cat_file_data);
+ else {
+ at = parse_ref_filter_atom(format, valid_atom,
+ ARRAY_SIZE(valid_atom), sp + 2, ep);
+ if (skip_prefix(used_atom[at].name, "color:", &color))
+ format->need_color_reset_at_eol = !!strcmp(color, "reset");
+ }
+
+ cp = ep + 1;
}
if (format->need_color_reset_at_eol && !want_color(format->use_color))
format->need_color_reset_at_eol = 0;
diff --git a/ref-filter.h b/ref-filter.h
index 17f2ac24d2739..52e07dbe6864a 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -80,12 +80,6 @@ struct expand_data {
const char *rest;
struct object_id delta_base_oid;
- /*
- * If mark_query is true, we do not expand anything, but rather
- * just mark the object_info with items we wish to query.
- */
- int mark_query;
-
/*
* Whether to split the input on whitespace before feeding it to
* get_sha1; this is decided during the mark_query phase based on
@@ -119,6 +113,12 @@ struct ref_format {
/* Internal state to ref-filter */
int need_color_reset_at_eol;
+
+ /*
+ * Helps to move all formatting logic from cat-file to ref-filter,
+ * hopefully would be reduced later.
+ */
+ struct expand_data *cat_file_data;
};
#define REF_FORMAT_INIT { NULL, 0, -1 }
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* [PATCH RFC v2 23/25] for-each-ref: tests for new atoms added
2018-02-05 11:27 ` [PATCH RFC v2 01/25] " Olga Telezhnaya
2018-02-05 11:27 ` [PATCH RFC v2 07/25] cat-file: start migrating formatting to ref-filter Olga Telezhnaya
@ 2018-02-05 11:27 ` Olga Telezhnaya
2018-02-05 11:27 ` [PATCH RFC v2 19/25] cat-file: reuse printing logic from ref-filter Olga Telezhnaya
` (22 subsequent siblings)
24 siblings, 0 replies; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-05 11:27 UTC (permalink / raw)
To: git
Add tests for new formatting atoms: rest, deltabase, objectsize:disk.
rest means nothing and we expand it into empty string.
We need this atom for cat-file command.
Have plans to support deltabase and objectsize:disk further
(as it is done in cat-file), now also expand it to empty string.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
t/t6300-for-each-ref.sh | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index c128dfc579079..eee656a6abba9 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -316,6 +316,24 @@ test_expect_success 'exercise strftime with odd fields' '
test_cmp expected actual
'
+test_expect_success 'Check format %(objectsize:disk) gives empty output ' '
+ echo >expected &&
+ git for-each-ref --format="%(objectsize:disk)" refs/heads >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'Check format %(rest) gives empty output ' '
+ echo >expected &&
+ git for-each-ref --format="%(rest)" refs/heads >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'Check format %(deltabase) gives empty output ' '
+ echo >expected &&
+ git for-each-ref --format="%(deltabase)" refs/heads >actual &&
+ test_cmp expected actual
+'
+
cat >expected <<\EOF
refs/heads/master
refs/remotes/origin/master
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* [PATCH RFC v2 19/25] cat-file: reuse printing logic from ref-filter
2018-02-05 11:27 ` [PATCH RFC v2 01/25] " Olga Telezhnaya
2018-02-05 11:27 ` [PATCH RFC v2 07/25] cat-file: start migrating formatting to ref-filter Olga Telezhnaya
2018-02-05 11:27 ` [PATCH RFC v2 23/25] for-each-ref: tests for new atoms added Olga Telezhnaya
@ 2018-02-05 11:27 ` Olga Telezhnaya
2018-02-05 11:27 ` [PATCH RFC v2 13/25] ref-filter: get rid of mark_atom_in_object_info() Olga Telezhnaya
` (21 subsequent siblings)
24 siblings, 0 replies; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-05 11:27 UTC (permalink / raw)
To: git
Reuse code from ref-filter to print resulting message.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
builtin/cat-file.c | 51 ++++-----------------------------------------------
ref-filter.c | 21 +++++++++++++++++++--
2 files changed, 23 insertions(+), 49 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 5b9869cdb9096..746b02ff150a7 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -176,45 +176,6 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
return 0;
}
-static int is_atom(const char *atom, const char *s, int slen)
-{
- int alen = strlen(atom);
- return alen == slen && !memcmp(atom, s, alen);
-}
-
-static void expand_atom(struct strbuf *sb, const char *atom, int len,
- struct ref_array_item *item)
-{
- if (is_atom("objectname", atom, len))
- strbuf_addstr(sb, oid_to_hex(&item->oid));
- else if (is_atom("objecttype", atom, len))
- strbuf_addstr(sb, typename(item->type));
- else if (is_atom("objectsize", atom, len))
- strbuf_addf(sb, "%lu", item->size);
- else if (is_atom("objectsize:disk", atom, len))
- strbuf_addf(sb, "%"PRIuMAX, (uintmax_t)item->disk_size);
- else if (is_atom("rest", atom, len)) {
- if (item->rest)
- strbuf_addstr(sb, item->rest);
- } else if (is_atom("deltabase", atom, len))
- strbuf_addstr(sb, oid_to_hex(item->delta_base_oid));
-}
-
-static size_t expand_format(struct strbuf *sb, const char *start, void *data)
-{
- const char *end;
- struct ref_array_item *item = data;
-
- if (*start != '(')
- return 0;
- end = strchr(start + 1, ')');
- if (!end)
- die("format element '%s' does not end in ')'", start);
-
- expand_atom(sb, start + 1, end - start - 1, item);
- return end - start + 1;
-}
-
static void batch_write(struct batch_options *opt, const void *data, int len)
{
if (opt->buffer_output) {
@@ -282,23 +243,19 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d
static void batch_object_write(const char *obj_name, struct batch_options *opt,
struct expand_data *data)
{
- struct strbuf buf = STRBUF_INIT;
struct ref_array_item item = {0};
item.oid = data->oid;
item.rest = data->rest;
item.objectname = obj_name;
- if (populate_value(&item))
+ if (show_ref_array_item(&item, &opt->format))
return;
-
- data->type = item.type;
- strbuf_expand(&buf, opt->format.format, expand_format, &item);
- strbuf_addch(&buf, '\n');
- batch_write(opt, buf.buf, buf.len);
- strbuf_release(&buf);
+ if (!opt->buffer_output)
+ fflush(stdout);
if (opt->print_contents) {
+ data->type = item.type;
print_object_or_die(opt, data);
batch_write(opt, "\n", 1);
}
diff --git a/ref-filter.c b/ref-filter.c
index 3f3583ac515a5..e34580e8db508 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1466,7 +1466,21 @@ int populate_value(struct ref_array_item *ref)
name++;
}
- if (starts_with(name, "refname"))
+ if (is_cat) {
+ if (starts_with(name, "objectname"))
+ v->s = oid_to_hex(&ref->oid);
+ else if (starts_with(name, "objecttype"))
+ v->s = typename(ref->type);
+ else if (starts_with(name, "objectsize")) {
+ v->s = xstrfmt("%lu", ref->size);
+ } else if (starts_with(name, "objectsize:disk")) {
+ v->s = xstrfmt("%"PRIuMAX, (uintmax_t)ref->disk_size);
+ } else if (starts_with(name, "rest"))
+ v->s = ref->rest;
+ else if (starts_with(name, "deltabase"))
+ v->s = xstrdup(oid_to_hex(ref->delta_base_oid));
+ continue;
+ } else if (starts_with(name, "refname"))
refname = get_refname(atom, ref);
else if (starts_with(name, "symref"))
refname = get_symref(atom, ref);
@@ -2208,6 +2222,7 @@ int format_ref_array_item(struct ref_array_item *info,
{
const char *cp, *sp, *ep;
struct ref_formatting_state state = REF_FORMATTING_STATE_INIT;
+ int retval = 0;
state.quote_style = format->quote_style;
push_stack_element(&state.stack);
@@ -2224,6 +2239,8 @@ int format_ref_array_item(struct ref_array_item *info,
return -1;
atomv->handler(atomv, &state);
}
+ if (is_cat && strlen(format->format) == 0)
+ retval = check_and_fill_for_cat(info);
if (*cp) {
sp = cp + strlen(cp);
append_literal(cp, sp, &state);
@@ -2237,7 +2254,7 @@ int format_ref_array_item(struct ref_array_item *info,
die(_("format: %%(end) atom missing"));
strbuf_addbuf(final_buf, &state.stack->output);
pop_stack_element(&state.stack);
- return 0;
+ return retval;
}
int show_ref_array_item(struct ref_array_item *info,
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* [PATCH RFC v2 13/25] ref-filter: get rid of mark_atom_in_object_info()
2018-02-05 11:27 ` [PATCH RFC v2 01/25] " Olga Telezhnaya
` (2 preceding siblings ...)
2018-02-05 11:27 ` [PATCH RFC v2 19/25] cat-file: reuse printing logic from ref-filter Olga Telezhnaya
@ 2018-02-05 11:27 ` Olga Telezhnaya
2018-02-05 11:27 ` [PATCH RFC v2 22/25] ref-filter: work with objectsize:disk Olga Telezhnaya
` (20 subsequent siblings)
24 siblings, 0 replies; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-05 11:27 UTC (permalink / raw)
To: git
Remove mark_atom_in_object_info() and create same logic
in terms of ref-filter style.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
ref-filter.c | 45 +++++++++++++++++++++------------------------
1 file changed, 21 insertions(+), 24 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 3f92a27d98b6c..34a54db168265 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -255,13 +255,29 @@ static void objectname_atom_parser(const struct ref_format *format, struct used_
static void objectsize_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)
{
if (!arg)
- ; /* default to normal object size */
+ cat_file_info->info.sizep = &cat_file_info->size;
else if (!strcmp(arg, "disk"))
cat_file_info->info.disk_sizep = &cat_file_info->disk_size;
else
die(_("urecognized %%(objectsize) argument: %s"), arg);
}
+static void objecttype_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)
+{
+ if (!arg)
+ cat_file_info->info.typep = &cat_file_info->type;
+ else
+ die(_("urecognized %%(objecttype) argument: %s"), arg);
+}
+
+static void deltabase_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)
+{
+ if (!arg)
+ cat_file_info->info.delta_base_sha1 = cat_file_info->delta_base_oid.hash;
+ else
+ die(_("urecognized %%(deltabase) argument: %s"), arg);
+}
+
static void refname_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)
{
refname_atom_parser_internal(&atom->u.refname, arg, atom->name);
@@ -384,10 +400,10 @@ static struct valid_atom {
static struct valid_atom valid_cat_file_atom[] = {
{ "objectname" },
- { "objecttype" },
+ { "objecttype", FIELD_STR, objecttype_atom_parser },
{ "objectsize", FIELD_ULONG, objectsize_atom_parser },
{ "rest" },
- { "deltabase" },
+ { "deltabase", FIELD_STR, deltabase_atom_parser },
};
#define REF_FORMATTING_STATE_INIT { 0, NULL }
@@ -411,25 +427,6 @@ struct atom_value {
struct used_atom *atom;
};
-static int is_atom(const char *atom, const char *s, int slen)
-{
- int alen = strlen(atom);
- return alen == slen && !memcmp(atom, s, alen);
-}
-
-static void mark_atom_in_object_info(const char *atom, int len,
- struct expand_data *data)
-{
- if (is_atom("objecttype", atom, len))
- data->info.typep = &data->type;
- else if (is_atom("objectsize", atom, len))
- data->info.sizep = &data->size;
- else if (is_atom("rest", atom, len))
- data->split_on_whitespace = 1;
- else if (is_atom("deltabase", atom, len))
- data->info.delta_base_sha1 = data->delta_base_oid.hash;
-}
-
/*
* Used to parse format string and sort specifiers
*/
@@ -496,8 +493,8 @@ static int parse_ref_filter_atom(const struct ref_format *format,
need_tagged = 1;
if (!strcmp(valid_atom[i].name, "symref"))
need_symref = 1;
- if (cat_file_info)
- mark_atom_in_object_info(atom, atom_len, cat_file_info);
+ if (cat_file_info && !strcmp(valid_atom[i].name, "rest"))
+ cat_file_info->split_on_whitespace = 1;
return at;
}
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* [PATCH RFC v2 22/25] ref-filter: work with objectsize:disk
2018-02-05 11:27 ` [PATCH RFC v2 01/25] " Olga Telezhnaya
` (3 preceding siblings ...)
2018-02-05 11:27 ` [PATCH RFC v2 13/25] ref-filter: get rid of mark_atom_in_object_info() Olga Telezhnaya
@ 2018-02-05 11:27 ` Olga Telezhnaya
2018-02-05 11:27 ` [PATCH RFC v2 10/25] ref-filter: make populate_value() global Olga Telezhnaya
` (19 subsequent siblings)
24 siblings, 0 replies; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-05 11:27 UTC (permalink / raw)
To: git
Make a temporary solution for commands that could use
objectsize:disk atom.
It's better to fill it with value or give an error if there is no value
for this atom, but as a first solution we do dothing.
It means that if objectsize:disk is used, we put an empty string there.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
ref-filter.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index aa15dd0b4723e..b21358aea476b 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -826,8 +826,10 @@ static void grab_common_values(struct atom_value *val, int deref, struct object
v->value = sz;
v->s = xstrfmt("%lu", sz);
} else if (!strcmp(name, "objectsize:disk")) {
- v->value = cat_file_info.disk_size;
- v->s = xstrfmt("%"PRIuMAX, (uintmax_t)v->value);
+ if (is_cat) {
+ v->value = cat_file_info.disk_size;
+ v->s = xstrfmt("%"PRIuMAX, (uintmax_t)v->value);
+ }
} else if (deref)
grab_objectname(name, obj->oid.hash, v, &used_atom[i]);
}
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* [PATCH RFC v2 10/25] ref-filter: make populate_value() global
2018-02-05 11:27 ` [PATCH RFC v2 01/25] " Olga Telezhnaya
` (4 preceding siblings ...)
2018-02-05 11:27 ` [PATCH RFC v2 22/25] ref-filter: work with objectsize:disk Olga Telezhnaya
@ 2018-02-05 11:27 ` Olga Telezhnaya
2018-02-05 11:27 ` [PATCH RFC v2 02/25] ref-filter: add return value to some functions Olga Telezhnaya
` (18 subsequent siblings)
24 siblings, 0 replies; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-05 11:27 UTC (permalink / raw)
To: git
Make function global for further using in cat-file.
In the end of patch series this function becomes internal again,
so this is a part of middle step. cat-file would use more general
functions further.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
ref-filter.c | 2 +-
ref-filter.h | 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/ref-filter.c b/ref-filter.c
index 5c75259b1ab8c..4acd391b5dfac 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1407,7 +1407,7 @@ static const char *get_refname(struct used_atom *atom, struct ref_array_item *re
* Parse the object referred by ref, and grab needed value.
* Return 0 if everything was successful, -1 otherwise.
*/
-static int populate_value(struct ref_array_item *ref)
+int populate_value(struct ref_array_item *ref)
{
void *buf;
struct object *obj;
diff --git a/ref-filter.h b/ref-filter.h
index 781921d4e0978..e16ea2a990119 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -182,4 +182,7 @@ void setup_ref_filter_porcelain_msg(void);
void pretty_print_ref(const char *name, const unsigned char *sha1,
const struct ref_format *format);
+/* Fill the values of request and prepare all data for final string creation */
+int populate_value(struct ref_array_item *ref);
+
#endif /* REF_FILTER_H */
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* [PATCH RFC v2 02/25] ref-filter: add return value to some functions
2018-02-05 11:27 ` [PATCH RFC v2 01/25] " Olga Telezhnaya
` (5 preceding siblings ...)
2018-02-05 11:27 ` [PATCH RFC v2 10/25] ref-filter: make populate_value() global Olga Telezhnaya
@ 2018-02-05 11:27 ` Olga Telezhnaya
2018-02-05 11:27 ` [PATCH RFC v2 15/25] ref_filter: add is_atom_used function Olga Telezhnaya
` (17 subsequent siblings)
24 siblings, 0 replies; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-05 11:27 UTC (permalink / raw)
To: git
Add return flag to format_ref_array_item(), show_ref_array_item(),
get_ref_array_info() and populate_value() for further using.
Need it to handle situations when item is broken but we can not invoke
die() because we are in batch mode and all items need to be processed.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
ref-filter.c | 37 ++++++++++++++++++++++++-------------
ref-filter.h | 14 ++++++++++----
2 files changed, 34 insertions(+), 17 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index d04295e33448e..9ed5e66066a7a 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1356,8 +1356,9 @@ static const char *get_refname(struct used_atom *atom, struct ref_array_item *re
/*
* Parse the object referred by ref, and grab needed value.
+ * Return 0 if everything was successful, -1 otherwise.
*/
-static void populate_value(struct ref_array_item *ref)
+static int populate_value(struct ref_array_item *ref)
{
void *buf;
struct object *obj;
@@ -1482,7 +1483,7 @@ static void populate_value(struct ref_array_item *ref)
}
}
if (used_atom_cnt <= i)
- return;
+ return 0;
buf = get_obj(&ref->objectname, &obj, &size, &eaten);
if (!buf)
@@ -1501,7 +1502,7 @@ static void populate_value(struct ref_array_item *ref)
* object, we are done.
*/
if (!need_tagged || (obj->type != OBJ_TAG))
- return;
+ return 0;
/*
* If it is a tag object, see if we use a value that derefs
@@ -1525,19 +1526,24 @@ static void populate_value(struct ref_array_item *ref)
grab_values(ref->value, 1, obj, buf, size);
if (!eaten)
free(buf);
+
+ return 0;
}
/*
* Given a ref, return the value for the atom. This lazily gets value
* out of the object by calling populate value.
+ * Return 0 if everything was successful, -1 otherwise.
*/
-static void get_ref_atom_value(struct ref_array_item *ref, int atom, struct atom_value **v)
+static int get_ref_atom_value(struct ref_array_item *ref, int atom, struct atom_value **v)
{
+ int retval = 0;
if (!ref->value) {
- populate_value(ref);
+ retval = populate_value(ref);
fill_missing_values(ref->value);
}
*v = &ref->value[atom];
+ return retval;
}
/*
@@ -2122,7 +2128,7 @@ static void append_literal(const char *cp, const char *ep, struct ref_formatting
}
}
-void format_ref_array_item(struct ref_array_item *info,
+int format_ref_array_item(struct ref_array_item *info,
const struct ref_format *format,
struct strbuf *final_buf)
{
@@ -2138,9 +2144,10 @@ void format_ref_array_item(struct ref_array_item *info,
ep = strchr(sp, ')');
if (cp < sp)
append_literal(cp, sp, &state);
- get_ref_atom_value(info,
- parse_ref_filter_atom(format, sp + 2, ep),
- &atomv);
+ if (get_ref_atom_value(info,
+ parse_ref_filter_atom(format, sp + 2, ep),
+ &atomv))
+ return -1;
atomv->handler(atomv, &state);
}
if (*cp) {
@@ -2156,17 +2163,21 @@ void format_ref_array_item(struct ref_array_item *info,
die(_("format: %%(end) atom missing"));
strbuf_addbuf(final_buf, &state.stack->output);
pop_stack_element(&state.stack);
+ return 0;
}
-void show_ref_array_item(struct ref_array_item *info,
+int show_ref_array_item(struct ref_array_item *info,
const struct ref_format *format)
{
struct strbuf final_buf = STRBUF_INIT;
+ int retval = format_ref_array_item(info, format, &final_buf);
- format_ref_array_item(info, format, &final_buf);
- fwrite(final_buf.buf, 1, final_buf.len, stdout);
+ if (!retval) {
+ fwrite(final_buf.buf, 1, final_buf.len, stdout);
+ putchar('\n');
+ }
strbuf_release(&final_buf);
- putchar('\n');
+ return retval;
}
void pretty_print_ref(const char *name, const unsigned char *sha1,
diff --git a/ref-filter.h b/ref-filter.h
index 0d98342b34319..b75c8ac45248e 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -109,12 +109,18 @@ void ref_array_clear(struct ref_array *array);
int verify_ref_format(struct ref_format *format);
/* Sort the given ref_array as per the ref_sorting provided */
void ref_array_sort(struct ref_sorting *sort, struct ref_array *array);
-/* Based on the given format and quote_style, fill the strbuf */
-void format_ref_array_item(struct ref_array_item *info,
+/*
+ * Based on the given format and quote_style, fill the strbuf.
+ * Return 0 if everything was successful, -1 otherwise (and strbuf remains empty)
+ */
+int format_ref_array_item(struct ref_array_item *info,
const struct ref_format *format,
struct strbuf *final_buf);
-/* Print the ref using the given format and quote_style */
-void show_ref_array_item(struct ref_array_item *info, const struct ref_format *format);
+/*
+ * Print the ref using the given format and quote_style.
+ * Return 0 if everything was successful, -1 otherwise.
+ */
+int show_ref_array_item(struct ref_array_item *info, const struct ref_format *format);
/* Parse a single sort specifier and add it to the list */
void parse_ref_sorting(struct ref_sorting **sorting_tail, const char *atom);
/* Callback function for parsing the sort option */
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* [PATCH RFC v2 15/25] ref_filter: add is_atom_used function
2018-02-05 11:27 ` [PATCH RFC v2 01/25] " Olga Telezhnaya
` (6 preceding siblings ...)
2018-02-05 11:27 ` [PATCH RFC v2 02/25] ref-filter: add return value to some functions Olga Telezhnaya
@ 2018-02-05 11:27 ` Olga Telezhnaya
2018-02-05 11:27 ` [PATCH RFC v2 20/25] ref-filter: make populate_value() internal again Olga Telezhnaya
` (16 subsequent siblings)
24 siblings, 0 replies; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-05 11:27 UTC (permalink / raw)
To: git
Delete all items related to split_on_whitespace from ref-filter
and add new function for handling the logic.
Now cat-file could invoke that function to implementing its logic.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
builtin/cat-file.c | 8 +++-----
ref-filter.c | 17 +++++++++++++++--
ref-filter.h | 10 +++-------
3 files changed, 21 insertions(+), 14 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index e8e788f41b890..a55138f1fd1d1 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -382,8 +382,7 @@ static int batch_objects(struct batch_options *opt)
{
struct strbuf buf = STRBUF_INIT;
struct expand_data data;
- int save_warning;
- int retval = 0;
+ int save_warning, is_rest, retval = 0;
if (!opt->format.format)
opt->format.format = "%(objectname) %(objecttype) %(objectsize)";
@@ -397,8 +396,6 @@ static int batch_objects(struct batch_options *opt)
opt->format.cat_file_data = &data;
opt->format.is_cat = 1;
verify_ref_format(&opt->format);
- if (opt->cmdmode)
- data.split_on_whitespace = 1;
if (opt->all_objects) {
struct object_info empty = OBJECT_INFO_INIT;
@@ -437,9 +434,10 @@ static int batch_objects(struct batch_options *opt)
*/
save_warning = warn_on_object_refname_ambiguity;
warn_on_object_refname_ambiguity = 0;
+ is_rest = opt->cmdmode || is_atom_used(&opt->format, "rest");
while (strbuf_getline(&buf, stdin) != EOF) {
- if (data.split_on_whitespace) {
+ if (is_rest) {
/*
* Split at first whitespace, tying off the beginning
* of the string and saving the remainder (or NULL) in
diff --git a/ref-filter.c b/ref-filter.c
index 91290b62450b3..bbcd507d179a9 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -494,8 +494,6 @@ static int parse_ref_filter_atom(const struct ref_format *format,
need_tagged = 1;
if (!strcmp(valid_atom[i].name, "symref"))
need_symref = 1;
- if (is_cat && !strcmp(valid_atom[i].name, "rest"))
- cat_file_info->split_on_whitespace = 1;
return at;
}
@@ -731,6 +729,21 @@ static const char *find_next(const char *cp)
return NULL;
}
+/* Search for atom in given format. */
+int is_atom_used(const struct ref_format *format, const char *atom)
+{
+ const char *cp, *sp;
+ for (cp = format->format; *cp && (sp = find_next(cp)); ) {
+ const char *ep = strchr(sp, ')');
+ int atom_len = ep - sp - 2;
+ sp += 2;
+ if (atom_len == strlen(atom) && !memcmp(sp, atom, atom_len))
+ return 1;
+ cp = ep + 1;
+ }
+ return 0;
+}
+
/*
* Make sure the format string is well formed, and parse out
* the used atoms.
diff --git a/ref-filter.h b/ref-filter.h
index 69271e8c39f40..f590e5d694df4 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -86,13 +86,6 @@ struct expand_data {
const char *rest;
struct object_id delta_base_oid;
- /*
- * Whether to split the input on whitespace before feeding it to
- * get_sha1; this is decided during the mark_query phase based on
- * whether we have a %(rest) token in our format.
- */
- int split_on_whitespace;
-
/*
* After a mark_query run, this object_info is set up to be
* passed to sha1_object_info_extended. It will point to the data
@@ -187,4 +180,7 @@ void pretty_print_ref(const char *name, const unsigned char *sha1,
/* Fill the values of request and prepare all data for final string creation */
int populate_value(struct ref_array_item *ref);
+/* Search for atom in given format. */
+int is_atom_used(const struct ref_format *format, const char *atom);
+
#endif /* REF_FILTER_H */
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* [PATCH RFC v2 20/25] ref-filter: make populate_value() internal again
2018-02-05 11:27 ` [PATCH RFC v2 01/25] " Olga Telezhnaya
` (7 preceding siblings ...)
2018-02-05 11:27 ` [PATCH RFC v2 15/25] ref_filter: add is_atom_used function Olga Telezhnaya
@ 2018-02-05 11:27 ` Olga Telezhnaya
2018-02-05 11:27 ` [PATCH RFC v2 16/25] cat-file: move skip_object_info into ref-filter Olga Telezhnaya
` (15 subsequent siblings)
24 siblings, 0 replies; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-05 11:27 UTC (permalink / raw)
To: git
Remove populate_value() from header file. We needed that
for interim step, now it could be returned back.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
ref-filter.c | 2 +-
ref-filter.h | 3 ---
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index e34580e8db508..70c685851466b 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1429,7 +1429,7 @@ static int check_and_fill_for_cat(struct ref_array_item *ref)
* Parse the object referred by ref, and grab needed value.
* Return 0 if everything was successful, -1 otherwise.
*/
-int populate_value(struct ref_array_item *ref)
+static int populate_value(struct ref_array_item *ref)
{
void *buf;
struct object *obj;
diff --git a/ref-filter.h b/ref-filter.h
index 244a27bfc4e12..c0edb17aa404a 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -177,9 +177,6 @@ void setup_ref_filter_porcelain_msg(void);
void pretty_print_ref(const char *name, const unsigned char *sha1,
const struct ref_format *format);
-/* Fill the values of request and prepare all data for final string creation */
-int populate_value(struct ref_array_item *ref);
-
/* Search for atom in given format. */
int is_atom_used(const struct ref_format *format, const char *atom);
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* [PATCH RFC v2 16/25] cat-file: move skip_object_info into ref-filter
2018-02-05 11:27 ` [PATCH RFC v2 01/25] " Olga Telezhnaya
` (8 preceding siblings ...)
2018-02-05 11:27 ` [PATCH RFC v2 20/25] ref-filter: make populate_value() internal again Olga Telezhnaya
@ 2018-02-05 11:27 ` Olga Telezhnaya
2018-02-05 11:27 ` [PATCH RFC v2 18/25] ref-filter: make valid_atom general again Olga Telezhnaya
` (14 subsequent siblings)
24 siblings, 0 replies; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-05 11:27 UTC (permalink / raw)
To: git
Move logic related to skip_object_info into ref-filter,
so that cat-file does not use that field at all.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
builtin/cat-file.c | 7 +------
ref-filter.c | 5 +++++
ref-filter.h | 1 +
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index a55138f1fd1d1..37adf626d0e55 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -395,14 +395,9 @@ static int batch_objects(struct batch_options *opt)
memset(&data, 0, sizeof(data));
opt->format.cat_file_data = &data;
opt->format.is_cat = 1;
+ opt->format.all_objects = opt->all_objects;
verify_ref_format(&opt->format);
- if (opt->all_objects) {
- struct object_info empty = OBJECT_INFO_INIT;
- if (!memcmp(&data.info, &empty, sizeof(empty)))
- data.skip_object_info = 1;
- }
-
/*
* If we are printing out the object, then always fill in the type,
* since we will want to decide whether or not to stream.
diff --git a/ref-filter.c b/ref-filter.c
index bbcd507d179a9..7dcd36cd2cddc 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -777,6 +777,11 @@ int verify_ref_format(struct ref_format *format)
}
if (format->need_color_reset_at_eol && !want_color(format->use_color))
format->need_color_reset_at_eol = 0;
+ if (is_cat && format->all_objects) {
+ struct object_info empty = OBJECT_INFO_INIT;
+ if (!memcmp(&cat_file_info->info, &empty, sizeof(empty)))
+ cat_file_info->skip_object_info = 1;
+ }
return 0;
}
diff --git a/ref-filter.h b/ref-filter.h
index f590e5d694df4..e882eb5126118 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -119,6 +119,7 @@ struct ref_format {
*/
struct expand_data *cat_file_data;
int is_cat;
+ int all_objects;
};
#define REF_FORMAT_INIT { NULL, 0, -1 }
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* [PATCH RFC v2 18/25] ref-filter: make valid_atom general again
2018-02-05 11:27 ` [PATCH RFC v2 01/25] " Olga Telezhnaya
` (9 preceding siblings ...)
2018-02-05 11:27 ` [PATCH RFC v2 16/25] cat-file: move skip_object_info into ref-filter Olga Telezhnaya
@ 2018-02-05 11:27 ` Olga Telezhnaya
2018-02-05 11:27 ` [PATCH RFC v2 08/25] ref-filter: reuse parse_ref_filter_atom() Olga Telezhnaya
` (13 subsequent siblings)
24 siblings, 0 replies; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-05 11:27 UTC (permalink / raw)
To: git
Stop using valid_cat_file_atom, making the code more general.
Further commits will contain some tests, docs and
support of new features.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
ref-filter.c | 34 +++++++++-------------------------
1 file changed, 9 insertions(+), 25 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index a65a90790fd2c..3f3583ac515a5 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -359,8 +359,8 @@ static struct valid_atom {
void (*parser)(const struct ref_format *format, struct used_atom *atom, const char *arg);
} valid_atom[] = {
{ "refname" , FIELD_STR, refname_atom_parser },
- { "objecttype" },
- { "objectsize", FIELD_ULONG },
+ { "objecttype", FIELD_STR, objecttype_atom_parser },
+ { "objectsize", FIELD_ULONG, objectsize_atom_parser },
{ "objectname", FIELD_STR, objectname_atom_parser },
{ "tree" },
{ "parent" },
@@ -397,12 +397,6 @@ static struct valid_atom {
{ "if", FIELD_STR, if_atom_parser },
{ "then" },
{ "else" },
-};
-
-static struct valid_atom valid_cat_file_atom[] = {
- { "objectname" },
- { "objecttype", FIELD_STR, objecttype_atom_parser },
- { "objectsize", FIELD_ULONG, objectsize_atom_parser },
{ "rest" },
{ "deltabase", FIELD_STR, deltabase_atom_parser },
};
@@ -432,7 +426,6 @@ struct atom_value {
* Used to parse format string and sort specifiers
*/
static int parse_ref_filter_atom(const struct ref_format *format,
- const struct valid_atom *valid_atom, int n_atoms,
const char *atom, const char *ep)
{
const char *sp;
@@ -462,13 +455,13 @@ static int parse_ref_filter_atom(const struct ref_format *format,
atom_len = (arg ? arg : ep) - sp;
/* Is the atom a valid one? */
- for (i = 0; i < n_atoms; i++) {
+ for (i = 0; i < ARRAY_SIZE(valid_atom); i++) {
int len = strlen(valid_atom[i].name);
if (len == atom_len && !memcmp(valid_atom[i].name, sp, len))
break;
}
- if (n_atoms <= i)
+ if (ARRAY_SIZE(valid_atom) <= i)
die(_("unknown field name: %.*s"), (int)(ep-atom), atom);
/* Add it in, including the deref prefix */
@@ -762,15 +755,9 @@ int verify_ref_format(struct ref_format *format)
return error(_("malformed format string %s"), sp);
/* sp points at "%(" and ep points at the closing ")" */
- if (is_cat)
- at = parse_ref_filter_atom(format, valid_cat_file_atom,
- ARRAY_SIZE(valid_cat_file_atom), sp + 2, ep);
- else {
- at = parse_ref_filter_atom(format, valid_atom,
- ARRAY_SIZE(valid_atom), sp + 2, ep);
- if (skip_prefix(used_atom[at].name, "color:", &color))
- format->need_color_reset_at_eol = !!strcmp(color, "reset");
- }
+ at = parse_ref_filter_atom(format, sp + 2, ep);
+ if (skip_prefix(used_atom[at].name, "color:", &color))
+ format->need_color_reset_at_eol = !!strcmp(color, "reset");
cp = ep + 1;
}
@@ -2232,9 +2219,7 @@ int format_ref_array_item(struct ref_array_item *info,
if (cp < sp)
append_literal(cp, sp, &state);
if (get_ref_atom_value(info,
- parse_ref_filter_atom(format, valid_atom,
- ARRAY_SIZE(valid_atom),
- sp + 2, ep),
+ parse_ref_filter_atom(format, sp + 2, ep),
&atomv))
return -1;
atomv->handler(atomv, &state);
@@ -2287,8 +2272,7 @@ static int parse_sorting_atom(const char *atom)
*/
struct ref_format dummy = REF_FORMAT_INIT;
const char *end = atom + strlen(atom);
- return parse_ref_filter_atom(&dummy, valid_atom,
- ARRAY_SIZE(valid_atom), atom, end);
+ return parse_ref_filter_atom(&dummy, atom, end);
}
/* If no sorting option is given, use refname to sort as default */
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* [PATCH RFC v2 08/25] ref-filter: reuse parse_ref_filter_atom()
2018-02-05 11:27 ` [PATCH RFC v2 01/25] " Olga Telezhnaya
` (10 preceding siblings ...)
2018-02-05 11:27 ` [PATCH RFC v2 18/25] ref-filter: make valid_atom general again Olga Telezhnaya
@ 2018-02-05 11:27 ` Olga Telezhnaya
2018-02-05 11:27 ` [PATCH RFC v2 17/25] ref-filter: make cat_file_info independent Olga Telezhnaya
` (12 subsequent siblings)
24 siblings, 0 replies; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-05 11:27 UTC (permalink / raw)
To: git
Continue migrating formatting logic from cat-file to ref-filter.
Reuse parse_ref_filter_atom() for unifying all processes in ref-filter
and further removing of mark_atom_in_object_info().
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
ref-filter.c | 34 +++++++++++++++++++++++++---------
1 file changed, 25 insertions(+), 9 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index ff87e632f463c..5c75259b1ab8c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -100,6 +100,7 @@ static struct used_atom {
} u;
} *used_atom;
static int used_atom_cnt, need_tagged, need_symref;
+struct expand_data *cat_file_info;
static void color_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *color_value)
{
@@ -251,6 +252,16 @@ static void objectname_atom_parser(const struct ref_format *format, struct used_
die(_("unrecognized %%(objectname) argument: %s"), arg);
}
+static void objectsize_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)
+{
+ if (!arg)
+ ; /* default to normal object size */
+ else if (!strcmp(arg, "disk"))
+ cat_file_info->info.disk_sizep = &cat_file_info->disk_size;
+ else
+ die(_("urecognized %%(objectsize) argument: %s"), arg);
+}
+
static void refname_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)
{
refname_atom_parser_internal(&atom->u.refname, arg, atom->name);
@@ -371,6 +382,14 @@ static struct valid_atom {
{ "else" },
};
+static struct valid_atom valid_cat_file_atom[] = {
+ { "objectname" },
+ { "objecttype" },
+ { "objectsize", FIELD_ULONG, objectsize_atom_parser },
+ { "rest" },
+ { "deltabase" },
+};
+
#define REF_FORMATTING_STATE_INIT { 0, NULL }
struct ref_formatting_stack {
@@ -401,20 +420,14 @@ static int is_atom(const char *atom, const char *s, int slen)
static void mark_atom_in_object_info(const char *atom, int len,
struct expand_data *data)
{
- if (is_atom("objectname", atom, len))
- ; /* do nothing */
- else if (is_atom("objecttype", atom, len))
+ if (is_atom("objecttype", atom, len))
data->info.typep = &data->type;
else if (is_atom("objectsize", atom, len))
data->info.sizep = &data->size;
- else if (is_atom("objectsize:disk", atom, len))
- data->info.disk_sizep = &data->disk_size;
else if (is_atom("rest", atom, len))
data->split_on_whitespace = 1;
else if (is_atom("deltabase", atom, len))
data->info.delta_base_sha1 = data->delta_base_oid.hash;
- else
- die("unknown format element: %.*s", len, atom);
}
/*
@@ -483,6 +496,8 @@ static int parse_ref_filter_atom(const struct ref_format *format,
need_tagged = 1;
if (!strcmp(valid_atom[i].name, "symref"))
need_symref = 1;
+ if (cat_file_info)
+ mark_atom_in_object_info(atom, atom_len, cat_file_info);
return at;
}
@@ -726,6 +741,7 @@ int verify_ref_format(struct ref_format *format)
{
const char *cp, *sp;
+ cat_file_info = format->cat_file_data;
format->need_color_reset_at_eol = 0;
for (cp = format->format; *cp && (sp = find_next(cp)); ) {
const char *color, *ep = strchr(sp, ')');
@@ -736,8 +752,8 @@ int verify_ref_format(struct ref_format *format)
/* sp points at "%(" and ep points at the closing ")" */
if (format->cat_file_data)
- mark_atom_in_object_info(sp + 2, ep - sp - 2,
- format->cat_file_data);
+ at = parse_ref_filter_atom(format, valid_cat_file_atom,
+ ARRAY_SIZE(valid_cat_file_atom), sp + 2, ep);
else {
at = parse_ref_filter_atom(format, valid_atom,
ARRAY_SIZE(valid_atom), sp + 2, ep);
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* [PATCH RFC v2 17/25] ref-filter: make cat_file_info independent
2018-02-05 11:27 ` [PATCH RFC v2 01/25] " Olga Telezhnaya
` (11 preceding siblings ...)
2018-02-05 11:27 ` [PATCH RFC v2 08/25] ref-filter: reuse parse_ref_filter_atom() Olga Telezhnaya
@ 2018-02-05 11:27 ` Olga Telezhnaya
2018-02-05 11:27 ` [PATCH RFC v2 09/25] cat-file: start use ref_array_item struct Olga Telezhnaya
` (11 subsequent siblings)
24 siblings, 0 replies; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-05 11:27 UTC (permalink / raw)
To: git
Remove connection between expand_data variable
in cat-file and in ref-filter.
It will help further to get rid of using expand_data in cat-file.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
builtin/cat-file.c | 2 +-
ref-filter.c | 29 +++++++++++++++--------------
ref-filter.h | 1 -
3 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 37adf626d0e55..5b9869cdb9096 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -292,6 +292,7 @@ static void batch_object_write(const char *obj_name, struct batch_options *opt,
if (populate_value(&item))
return;
+ data->type = item.type;
strbuf_expand(&buf, opt->format.format, expand_format, &item);
strbuf_addch(&buf, '\n');
batch_write(opt, buf.buf, buf.len);
@@ -393,7 +394,6 @@ static int batch_objects(struct batch_options *opt)
* object.
*/
memset(&data, 0, sizeof(data));
- opt->format.cat_file_data = &data;
opt->format.is_cat = 1;
opt->format.all_objects = opt->all_objects;
verify_ref_format(&opt->format);
diff --git a/ref-filter.c b/ref-filter.c
index 7dcd36cd2cddc..a65a90790fd2c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -100,7 +100,7 @@ static struct used_atom {
} u;
} *used_atom;
static int used_atom_cnt, need_tagged, need_symref;
-struct expand_data *cat_file_info;
+struct expand_data cat_file_info;
static int is_cat = 0;
static void color_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *color_value)
@@ -256,9 +256,9 @@ static void objectname_atom_parser(const struct ref_format *format, struct used_
static void objectsize_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)
{
if (!arg)
- cat_file_info->info.sizep = &cat_file_info->size;
+ cat_file_info.info.sizep = &cat_file_info.size;
else if (!strcmp(arg, "disk"))
- cat_file_info->info.disk_sizep = &cat_file_info->disk_size;
+ cat_file_info.info.disk_sizep = &cat_file_info.disk_size;
else
die(_("urecognized %%(objectsize) argument: %s"), arg);
}
@@ -266,7 +266,7 @@ static void objectsize_atom_parser(const struct ref_format *format, struct used_
static void objecttype_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)
{
if (!arg)
- cat_file_info->info.typep = &cat_file_info->type;
+ cat_file_info.info.typep = &cat_file_info.type;
else
die(_("urecognized %%(objecttype) argument: %s"), arg);
}
@@ -274,7 +274,7 @@ static void objecttype_atom_parser(const struct ref_format *format, struct used_
static void deltabase_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)
{
if (!arg)
- cat_file_info->info.delta_base_sha1 = cat_file_info->delta_base_oid.hash;
+ cat_file_info.info.delta_base_sha1 = cat_file_info.delta_base_oid.hash;
else
die(_("urecognized %%(deltabase) argument: %s"), arg);
}
@@ -752,7 +752,6 @@ int verify_ref_format(struct ref_format *format)
{
const char *cp, *sp;
- cat_file_info = format->cat_file_data;
is_cat = format->is_cat;
format->need_color_reset_at_eol = 0;
for (cp = format->format; *cp && (sp = find_next(cp)); ) {
@@ -779,8 +778,8 @@ int verify_ref_format(struct ref_format *format)
format->need_color_reset_at_eol = 0;
if (is_cat && format->all_objects) {
struct object_info empty = OBJECT_INFO_INIT;
- if (!memcmp(&cat_file_info->info, &empty, sizeof(empty)))
- cat_file_info->skip_object_info = 1;
+ if (!memcmp(&cat_file_info.info, &empty, sizeof(empty)))
+ cat_file_info.skip_object_info = 1;
}
return 0;
}
@@ -1422,18 +1421,20 @@ static const char *get_refname(struct used_atom *atom, struct ref_array_item *re
static int check_and_fill_for_cat(struct ref_array_item *ref)
{
- if (!cat_file_info->skip_object_info &&
- sha1_object_info_extended(ref->oid.hash, &cat_file_info->info,
+ if (!cat_file_info.info.typep)
+ cat_file_info.info.typep = &cat_file_info.type;
+ if (!cat_file_info.skip_object_info &&
+ sha1_object_info_extended(ref->oid.hash, &cat_file_info.info,
OBJECT_INFO_LOOKUP_REPLACE) < 0) {
const char *e = ref->objectname;
printf("%s missing\n", e ? e : oid_to_hex(&ref->oid));
fflush(stdout);
return -1;
}
- ref->type = cat_file_info->type;
- ref->size = cat_file_info->size;
- ref->disk_size = cat_file_info->disk_size;
- ref->delta_base_oid = &cat_file_info->delta_base_oid;
+ ref->type = cat_file_info.type;
+ ref->size = cat_file_info.size;
+ ref->disk_size = cat_file_info.disk_size;
+ ref->delta_base_oid = &cat_file_info.delta_base_oid;
return 0;
}
diff --git a/ref-filter.h b/ref-filter.h
index e882eb5126118..244a27bfc4e12 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -117,7 +117,6 @@ struct ref_format {
* Helps to move all formatting logic from cat-file to ref-filter,
* hopefully would be reduced later.
*/
- struct expand_data *cat_file_data;
int is_cat;
int all_objects;
};
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* [PATCH RFC v2 09/25] cat-file: start use ref_array_item struct
2018-02-05 11:27 ` [PATCH RFC v2 01/25] " Olga Telezhnaya
` (12 preceding siblings ...)
2018-02-05 11:27 ` [PATCH RFC v2 17/25] ref-filter: make cat_file_info independent Olga Telezhnaya
@ 2018-02-05 11:27 ` Olga Telezhnaya
2018-02-05 11:27 ` [PATCH RFC v2 03/25] cat-file: reuse struct ref_format Olga Telezhnaya
` (10 subsequent siblings)
24 siblings, 0 replies; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-05 11:27 UTC (permalink / raw)
To: git
Moving from using expand_data to ref_array_item structure.
That helps us to reuse functions from ref-filter easier.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
builtin/cat-file.c | 32 ++++++++++++++++++++------------
ref-filter.h | 5 +++++
2 files changed, 25 insertions(+), 12 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 909412747cbd2..61b7acc79155d 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -183,27 +183,27 @@ static int is_atom(const char *atom, const char *s, int slen)
}
static void expand_atom(struct strbuf *sb, const char *atom, int len,
- struct expand_data *data)
+ struct ref_array_item *item)
{
if (is_atom("objectname", atom, len))
- strbuf_addstr(sb, oid_to_hex(&data->oid));
+ strbuf_addstr(sb, oid_to_hex(&item->objectname));
else if (is_atom("objecttype", atom, len))
- strbuf_addstr(sb, typename(data->type));
+ strbuf_addstr(sb, typename(item->type));
else if (is_atom("objectsize", atom, len))
- strbuf_addf(sb, "%lu", data->size);
+ strbuf_addf(sb, "%lu", item->size);
else if (is_atom("objectsize:disk", atom, len))
- strbuf_addf(sb, "%"PRIuMAX, (uintmax_t)data->disk_size);
+ strbuf_addf(sb, "%"PRIuMAX, (uintmax_t)item->disk_size);
else if (is_atom("rest", atom, len)) {
- if (data->rest)
- strbuf_addstr(sb, data->rest);
+ if (item->rest)
+ strbuf_addstr(sb, item->rest);
} else if (is_atom("deltabase", atom, len))
- strbuf_addstr(sb, oid_to_hex(&data->delta_base_oid));
+ strbuf_addstr(sb, oid_to_hex(item->delta_base_oid));
}
-static size_t expand_format(struct strbuf *sb, const char *start, void *vdata)
+static size_t expand_format(struct strbuf *sb, const char *start, void *data)
{
const char *end;
- struct expand_data *data = vdata;
+ struct ref_array_item *item = data;
if (*start != '(')
return 0;
@@ -211,7 +211,7 @@ static size_t expand_format(struct strbuf *sb, const char *start, void *vdata)
if (!end)
die("format element '%s' does not end in ')'", start);
- expand_atom(sb, start + 1, end - start - 1, data);
+ expand_atom(sb, start + 1, end - start - 1, item);
return end - start + 1;
}
@@ -283,6 +283,7 @@ static void batch_object_write(const char *obj_name, struct batch_options *opt,
struct expand_data *data)
{
struct strbuf buf = STRBUF_INIT;
+ struct ref_array_item item = {0};
if (!data->skip_object_info &&
sha1_object_info_extended(data->oid.hash, &data->info,
@@ -293,7 +294,14 @@ static void batch_object_write(const char *obj_name, struct batch_options *opt,
return;
}
- strbuf_expand(&buf, opt->format.format, expand_format, data);
+ item.objectname = data->oid;
+ item.type = data->type;
+ item.size = data->size;
+ item.disk_size = data->disk_size;
+ item.rest = data->rest;
+ item.delta_base_oid = &data->delta_base_oid;
+
+ strbuf_expand(&buf, opt->format.format, expand_format, &item);
strbuf_addch(&buf, '\n');
batch_write(opt, buf.buf, buf.len);
strbuf_release(&buf);
diff --git a/ref-filter.h b/ref-filter.h
index 52e07dbe6864a..781921d4e0978 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -40,6 +40,11 @@ struct ref_array_item {
const char *symref;
struct commit *commit;
struct atom_value *value;
+ enum object_type type;
+ unsigned long size;
+ off_t disk_size;
+ const char *rest;
+ struct object_id *delta_base_oid;
char refname[FLEX_ARRAY];
};
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* [PATCH RFC v2 03/25] cat-file: reuse struct ref_format
2018-02-05 11:27 ` [PATCH RFC v2 01/25] " Olga Telezhnaya
` (13 preceding siblings ...)
2018-02-05 11:27 ` [PATCH RFC v2 09/25] cat-file: start use ref_array_item struct Olga Telezhnaya
@ 2018-02-05 11:27 ` Olga Telezhnaya
2018-02-05 11:27 ` [PATCH RFC v2 21/25] ref-filter: unifying formatting of cat-file opts Olga Telezhnaya
` (9 subsequent siblings)
24 siblings, 0 replies; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-05 11:27 UTC (permalink / raw)
To: git
Start using ref_format struct instead of simple char*.
Need that for further reusing of formatting logic from ref-filter.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
builtin/cat-file.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index f5fa4fd75af26..98fc5ec069a49 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -13,15 +13,16 @@
#include "tree-walk.h"
#include "sha1-array.h"
#include "packfile.h"
+#include "ref-filter.h"
struct batch_options {
+ struct ref_format format;
int enabled;
int follow_symlinks;
int print_contents;
int buffer_output;
int all_objects;
int cmdmode; /* may be 'w' or 'c' for --filters or --textconv */
- const char *format;
};
static const char *force_path;
@@ -348,7 +349,7 @@ static void batch_object_write(const char *obj_name, struct batch_options *opt,
return;
}
- strbuf_expand(&buf, opt->format, expand_format, data);
+ strbuf_expand(&buf, opt->format.format, expand_format, data);
strbuf_addch(&buf, '\n');
batch_write(opt, buf.buf, buf.len);
strbuf_release(&buf);
@@ -441,8 +442,8 @@ static int batch_objects(struct batch_options *opt)
int save_warning;
int retval = 0;
- if (!opt->format)
- opt->format = "%(objectname) %(objecttype) %(objectsize)";
+ if (!opt->format.format)
+ opt->format.format = "%(objectname) %(objecttype) %(objectsize)";
/*
* Expand once with our special mark_query flag, which will prime the
@@ -451,7 +452,7 @@ static int batch_objects(struct batch_options *opt)
*/
memset(&data, 0, sizeof(data));
data.mark_query = 1;
- strbuf_expand(&buf, opt->format, expand_format, &data);
+ strbuf_expand(&buf, opt->format.format, expand_format, &data);
data.mark_query = 0;
if (opt->cmdmode)
data.split_on_whitespace = 1;
@@ -543,7 +544,7 @@ static int batch_option_callback(const struct option *opt,
bo->enabled = 1;
bo->print_contents = !strcmp(opt->long_name, "batch");
- bo->format = arg;
+ bo->format.format = arg;
return 0;
}
@@ -552,7 +553,7 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
{
int opt = 0;
const char *exp_type = NULL, *obj_name = NULL;
- struct batch_options batch = {0};
+ struct batch_options batch = { REF_FORMAT_INIT };
int unknown_type = 0;
const struct option options[] = {
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* [PATCH RFC v2 21/25] ref-filter: unifying formatting of cat-file opts
2018-02-05 11:27 ` [PATCH RFC v2 01/25] " Olga Telezhnaya
` (14 preceding siblings ...)
2018-02-05 11:27 ` [PATCH RFC v2 03/25] cat-file: reuse struct ref_format Olga Telezhnaya
@ 2018-02-05 11:27 ` Olga Telezhnaya
2018-02-05 11:27 ` [PATCH RFC v2 24/25] cat-file: tests for new atoms added Olga Telezhnaya
` (8 subsequent siblings)
24 siblings, 0 replies; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-05 11:27 UTC (permalink / raw)
To: git
cat-file options are now filled by general logic.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
ref-filter.c | 31 ++++++++++++++-----------------
1 file changed, 14 insertions(+), 17 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 70c685851466b..aa15dd0b4723e 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -825,8 +825,10 @@ static void grab_common_values(struct atom_value *val, int deref, struct object
else if (!strcmp(name, "objectsize")) {
v->value = sz;
v->s = xstrfmt("%lu", sz);
- }
- else if (deref)
+ } else if (!strcmp(name, "objectsize:disk")) {
+ v->value = cat_file_info.disk_size;
+ v->s = xstrfmt("%"PRIuMAX, (uintmax_t)v->value);
+ } else if (deref)
grab_objectname(name, obj->oid.hash, v, &used_atom[i]);
}
}
@@ -1466,21 +1468,7 @@ static int populate_value(struct ref_array_item *ref)
name++;
}
- if (is_cat) {
- if (starts_with(name, "objectname"))
- v->s = oid_to_hex(&ref->oid);
- else if (starts_with(name, "objecttype"))
- v->s = typename(ref->type);
- else if (starts_with(name, "objectsize")) {
- v->s = xstrfmt("%lu", ref->size);
- } else if (starts_with(name, "objectsize:disk")) {
- v->s = xstrfmt("%"PRIuMAX, (uintmax_t)ref->disk_size);
- } else if (starts_with(name, "rest"))
- v->s = ref->rest;
- else if (starts_with(name, "deltabase"))
- v->s = xstrdup(oid_to_hex(ref->delta_base_oid));
- continue;
- } else if (starts_with(name, "refname"))
+ if (starts_with(name, "refname"))
refname = get_refname(atom, ref);
else if (starts_with(name, "symref"))
refname = get_symref(atom, ref);
@@ -1536,6 +1524,15 @@ static int populate_value(struct ref_array_item *ref)
else
v->s = " ";
continue;
+ } else if (starts_with(name, "rest")) {
+ v->s = ref->rest ? ref->rest : "";
+ continue;
+ } else if (starts_with(name, "deltabase")) {
+ if (ref->delta_base_oid)
+ v->s = xstrdup(oid_to_hex(ref->delta_base_oid));
+ else
+ v->s = "";
+ continue;
} else if (starts_with(name, "align")) {
v->handler = align_atom_handler;
continue;
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* [PATCH RFC v2 24/25] cat-file: tests for new atoms added
2018-02-05 11:27 ` [PATCH RFC v2 01/25] " Olga Telezhnaya
` (15 preceding siblings ...)
2018-02-05 11:27 ` [PATCH RFC v2 21/25] ref-filter: unifying formatting of cat-file opts Olga Telezhnaya
@ 2018-02-05 11:27 ` Olga Telezhnaya
2018-02-05 11:27 ` [PATCH RFC v2 12/25] cat-file: start reusing populate_value() Olga Telezhnaya
` (7 subsequent siblings)
24 siblings, 0 replies; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-05 11:27 UTC (permalink / raw)
To: git
Add some tests for new formatting atoms from ref-filter.
Some of new atoms are supported automatically,
some of them are expanded into empty string
(because they are useless for some types of objects),
some of them could be supported later in other patches.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
t/t1006-cat-file.sh | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index b19f332694620..e72fcaf0e02c5 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -20,6 +20,19 @@ maybe_remove_timestamp () {
fi
}
+test_atom () {
+ name=$1
+ sha1=$2
+ atoms=$3
+ expected=$4
+
+ test_expect_success "$name" '
+ echo "$expected" >expect &&
+ echo $sha1 | git cat-file --batch-check="$atoms" >actual &&
+ test_cmp expect actual
+ '
+}
+
run_tests () {
type=$1
sha1=$2
@@ -119,6 +132,13 @@ $content"
maybe_remove_timestamp "$(cat actual.full)" $no_ts >actual &&
test_cmp expect actual
'
+
+ for atom in refname parent body trailers upstream push symref flag
+ do
+ test_atom "Check %($atom) gives empty output" "$sha1" "%($atom)" ""
+ done
+
+ test_atom "Check %(HEAD) gives only one space as output" "$sha1" '%(HEAD)' ' '
}
hello_content="Hello World"
@@ -140,6 +160,12 @@ test_expect_success '--batch-check without %(rest) considers whole line' '
test_cmp expect actual
'
+shortname=`echo $hello_sha1 | sed 's/^.\{0\}\(.\{7\}\).*/\1/'`
+test_atom 'Check format option %(objectname:short) works' "$hello_sha1" '%(objectname:short)' "$shortname"
+
+test_atom 'Check format option %(align) is not broken' \
+ "$hello_sha1" "%(align:8)%(objecttype)%(end)%(objectname)" "blob $hello_sha1"
+
tree_sha1=$(git write-tree)
tree_size=33
tree_pretty_content="100644 blob $hello_sha1 hello"
@@ -157,6 +183,17 @@ $commit_message"
run_tests 'commit' $commit_sha1 $commit_size "$commit_content" "$commit_content" 1
+test_atom "Check format option %(if) is not broken" "$commit_sha1" \
+ "%(if)%(author)%(then)%(objectname)%(end)" "$commit_sha1"
+test_atom "Check %(tree) works for commit" "$commit_sha1" "%(tree)" "$tree_sha1"
+test_atom "Check %(numparent) works for commit" "$commit_sha1" "%(numparent)" "0"
+test_atom "Check %(authorname) works for commit" "$commit_sha1" "%(authorname)" "$GIT_AUTHOR_NAME"
+test_atom "Check %(authoremail) works for commit" "$commit_sha1" "%(authoremail)" "<$GIT_AUTHOR_EMAIL>"
+test_atom "Check %(committername) works for commit" "$commit_sha1" "%(committername)" "$GIT_COMMITTER_NAME"
+test_atom "Check %(committeremail) works for commit" "$commit_sha1" "%(committeremail)" "<$GIT_COMMITTER_EMAIL>"
+test_atom "Check %(subject) works for commit" "$commit_sha1" "%(subject)" "$commit_message"
+test_atom "Check %(contents) works for commit" "$commit_sha1" "%(contents)" "$commit_message"
+
tag_header_without_timestamp="object $hello_sha1
type blob
tag hellotag
@@ -171,6 +208,17 @@ tag_size=$(strlen "$tag_content")
run_tests 'tag' $tag_sha1 $tag_size "$tag_content" "$tag_content" 1
+test_atom "Check %(object) works for tag" "$tag_sha1" "%(object)" "$hello_sha1"
+test_atom "Check %(type) works for tag" "$tag_sha1" "%(type)" "blob"
+test_atom "Check %(tag) works for tag" "$tag_sha1" "%(tag)" "hellotag"
+test_atom "Check %(taggername) works for tag" "$tag_sha1" "%(taggername)" "$GIT_COMMITTER_NAME"
+test_atom "Check %(taggeremail) works for tag" "$tag_sha1" "%(taggeremail)" "<$GIT_COMMITTER_EMAIL>"
+test_atom "Check %(subject) works for tag" "$tag_sha1" "%(subject)" "$tag_description"
+test_atom "Check %(contents) works for tag" "$tag_sha1" "%(contents)" "$tag_description"
+
+test_atom "Check %(color) gives no additional output" "$sha1" \
+ "%(objectname) %(color:green) %(objecttype)" "$sha1 $type"
+
test_expect_success \
"Reach a blob from a tag pointing to it" \
"test '$hello_content' = \"\$(git cat-file blob $tag_sha1)\""
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* [PATCH RFC v2 12/25] cat-file: start reusing populate_value()
2018-02-05 11:27 ` [PATCH RFC v2 01/25] " Olga Telezhnaya
` (16 preceding siblings ...)
2018-02-05 11:27 ` [PATCH RFC v2 24/25] cat-file: tests for new atoms added Olga Telezhnaya
@ 2018-02-05 11:27 ` Olga Telezhnaya
2018-02-05 11:27 ` [PATCH RFC v2 04/25] ref-filter: make valid_atom as function parameter Olga Telezhnaya
` (6 subsequent siblings)
24 siblings, 0 replies; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-05 11:27 UTC (permalink / raw)
To: git
Move logic related to getting object info from cat-file to ref-filter.
It will help to reuse whole formatting logic from ref-filter further.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
builtin/cat-file.c | 17 ++++-------------
ref-filter.c | 20 ++++++++++++++++++++
ref-filter.h | 1 +
3 files changed, 25 insertions(+), 13 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 367b1bd5802dc..179c955b86bd5 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -285,21 +285,12 @@ static void batch_object_write(const char *obj_name, struct batch_options *opt,
struct strbuf buf = STRBUF_INIT;
struct ref_array_item item = {0};
- if (!data->skip_object_info &&
- sha1_object_info_extended(data->oid.hash, &data->info,
- OBJECT_INFO_LOOKUP_REPLACE) < 0) {
- printf("%s missing\n",
- obj_name ? obj_name : oid_to_hex(&data->oid));
- fflush(stdout);
- return;
- }
-
item.oid = data->oid;
- item.type = data->type;
- item.size = data->size;
- item.disk_size = data->disk_size;
item.rest = data->rest;
- item.delta_base_oid = &data->delta_base_oid;
+ item.objectname = obj_name;
+
+ if (populate_value(&item))
+ return;
strbuf_expand(&buf, opt->format.format, expand_format, &item);
strbuf_addch(&buf, '\n');
diff --git a/ref-filter.c b/ref-filter.c
index d09ec1bde6d54..3f92a27d98b6c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1403,6 +1403,23 @@ static const char *get_refname(struct used_atom *atom, struct ref_array_item *re
return show_ref(&atom->u.refname, ref->refname);
}
+static int check_and_fill_for_cat(struct ref_array_item *ref)
+{
+ if (!cat_file_info->skip_object_info &&
+ sha1_object_info_extended(ref->oid.hash, &cat_file_info->info,
+ OBJECT_INFO_LOOKUP_REPLACE) < 0) {
+ const char *e = ref->objectname;
+ printf("%s missing\n", e ? e : oid_to_hex(&ref->oid));
+ fflush(stdout);
+ return -1;
+ }
+ ref->type = cat_file_info->type;
+ ref->size = cat_file_info->size;
+ ref->disk_size = cat_file_info->disk_size;
+ ref->delta_base_oid = &cat_file_info->delta_base_oid;
+ return 0;
+}
+
/*
* Parse the object referred by ref, and grab needed value.
* Return 0 if everything was successful, -1 otherwise.
@@ -1424,6 +1441,9 @@ int populate_value(struct ref_array_item *ref)
ref->symref = "";
}
+ if (cat_file_info && check_and_fill_for_cat(ref))
+ return -1;
+
/* Fill in specials first */
for (i = 0; i < used_atom_cnt; i++) {
struct used_atom *atom = &used_atom[i];
diff --git a/ref-filter.h b/ref-filter.h
index 87b026b8b76d0..5c6e019998716 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -45,6 +45,7 @@ struct ref_array_item {
off_t disk_size;
const char *rest;
struct object_id *delta_base_oid;
+ const char *objectname;
char refname[FLEX_ARRAY];
};
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* [PATCH RFC v2 04/25] ref-filter: make valid_atom as function parameter
2018-02-05 11:27 ` [PATCH RFC v2 01/25] " Olga Telezhnaya
` (17 preceding siblings ...)
2018-02-05 11:27 ` [PATCH RFC v2 12/25] cat-file: start reusing populate_value() Olga Telezhnaya
@ 2018-02-05 11:27 ` Olga Telezhnaya
2018-02-05 11:27 ` [PATCH RFC v2 14/25] ref-filter: add is_cat flag Olga Telezhnaya
` (5 subsequent siblings)
24 siblings, 0 replies; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-05 11:27 UTC (permalink / raw)
To: git
Make valid_atom as a function parameter,
there could be another variable further.
Need that for further reusing of formatting logic in cat-file.c.
We do not need to allow users to pass their own valid_atom variable in
global functions like verify_ref_format() because in the end we want to
have same set of valid atoms for all commands. But, as a first step
of migrating, I create further another version of valid_atom
for cat-file.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
ref-filter.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 9ed5e66066a7a..5e7ed0f338490 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -325,7 +325,7 @@ static void head_atom_parser(const struct ref_format *format, struct used_atom *
atom->u.head = resolve_refdup("HEAD", RESOLVE_REF_READING, NULL, NULL);
}
-static struct {
+static struct valid_atom {
const char *name;
cmp_type cmp_type;
void (*parser)(const struct ref_format *format, struct used_atom *atom, const char *arg);
@@ -396,6 +396,7 @@ struct atom_value {
* Used to parse format string and sort specifiers
*/
static int parse_ref_filter_atom(const struct ref_format *format,
+ const struct valid_atom *valid_atom, int n_atoms,
const char *atom, const char *ep)
{
const char *sp;
@@ -425,13 +426,13 @@ static int parse_ref_filter_atom(const struct ref_format *format,
atom_len = (arg ? arg : ep) - sp;
/* Is the atom a valid one? */
- for (i = 0; i < ARRAY_SIZE(valid_atom); i++) {
+ for (i = 0; i < n_atoms; i++) {
int len = strlen(valid_atom[i].name);
if (len == atom_len && !memcmp(valid_atom[i].name, sp, len))
break;
}
- if (ARRAY_SIZE(valid_atom) <= i)
+ if (n_atoms <= i)
die(_("unknown field name: %.*s"), (int)(ep-atom), atom);
/* Add it in, including the deref prefix */
@@ -708,7 +709,8 @@ int verify_ref_format(struct ref_format *format)
if (!ep)
return error(_("malformed format string %s"), sp);
/* sp points at "%(" and ep points at the closing ")" */
- at = parse_ref_filter_atom(format, sp + 2, ep);
+ at = parse_ref_filter_atom(format, valid_atom,
+ ARRAY_SIZE(valid_atom), sp + 2, ep);
cp = ep + 1;
if (skip_prefix(used_atom[at].name, "color:", &color))
@@ -2145,7 +2147,9 @@ int format_ref_array_item(struct ref_array_item *info,
if (cp < sp)
append_literal(cp, sp, &state);
if (get_ref_atom_value(info,
- parse_ref_filter_atom(format, sp + 2, ep),
+ parse_ref_filter_atom(format, valid_atom,
+ ARRAY_SIZE(valid_atom),
+ sp + 2, ep),
&atomv))
return -1;
atomv->handler(atomv, &state);
@@ -2198,7 +2202,8 @@ static int parse_sorting_atom(const char *atom)
*/
struct ref_format dummy = REF_FORMAT_INIT;
const char *end = atom + strlen(atom);
- return parse_ref_filter_atom(&dummy, atom, end);
+ return parse_ref_filter_atom(&dummy, valid_atom,
+ ARRAY_SIZE(valid_atom), atom, end);
}
/* If no sorting option is given, use refname to sort as default */
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* [PATCH RFC v2 14/25] ref-filter: add is_cat flag
2018-02-05 11:27 ` [PATCH RFC v2 01/25] " Olga Telezhnaya
` (18 preceding siblings ...)
2018-02-05 11:27 ` [PATCH RFC v2 04/25] ref-filter: make valid_atom as function parameter Olga Telezhnaya
@ 2018-02-05 11:27 ` Olga Telezhnaya
2018-02-05 21:19 ` Junio C Hamano
2018-02-05 11:27 ` [PATCH RFC v2 05/25] cat-file: move struct expand_data into ref-filter Olga Telezhnaya
` (4 subsequent siblings)
24 siblings, 1 reply; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-05 11:27 UTC (permalink / raw)
To: git
Add is_cat flag, further it helps to get rid of cat_file_data field
in ref_format.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
builtin/cat-file.c | 1 +
ref-filter.c | 8 +++++---
ref-filter.h | 1 +
3 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 179c955b86bd5..e8e788f41b890 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -395,6 +395,7 @@ static int batch_objects(struct batch_options *opt)
*/
memset(&data, 0, sizeof(data));
opt->format.cat_file_data = &data;
+ opt->format.is_cat = 1;
verify_ref_format(&opt->format);
if (opt->cmdmode)
data.split_on_whitespace = 1;
diff --git a/ref-filter.c b/ref-filter.c
index 34a54db168265..91290b62450b3 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -101,6 +101,7 @@ static struct used_atom {
} *used_atom;
static int used_atom_cnt, need_tagged, need_symref;
struct expand_data *cat_file_info;
+static int is_cat = 0;
static void color_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *color_value)
{
@@ -493,7 +494,7 @@ static int parse_ref_filter_atom(const struct ref_format *format,
need_tagged = 1;
if (!strcmp(valid_atom[i].name, "symref"))
need_symref = 1;
- if (cat_file_info && !strcmp(valid_atom[i].name, "rest"))
+ if (is_cat && !strcmp(valid_atom[i].name, "rest"))
cat_file_info->split_on_whitespace = 1;
return at;
}
@@ -739,6 +740,7 @@ int verify_ref_format(struct ref_format *format)
const char *cp, *sp;
cat_file_info = format->cat_file_data;
+ is_cat = format->is_cat;
format->need_color_reset_at_eol = 0;
for (cp = format->format; *cp && (sp = find_next(cp)); ) {
const char *color, *ep = strchr(sp, ')');
@@ -748,7 +750,7 @@ int verify_ref_format(struct ref_format *format)
return error(_("malformed format string %s"), sp);
/* sp points at "%(" and ep points at the closing ")" */
- if (format->cat_file_data)
+ if (is_cat)
at = parse_ref_filter_atom(format, valid_cat_file_atom,
ARRAY_SIZE(valid_cat_file_atom), sp + 2, ep);
else {
@@ -1438,7 +1440,7 @@ int populate_value(struct ref_array_item *ref)
ref->symref = "";
}
- if (cat_file_info && check_and_fill_for_cat(ref))
+ if (is_cat && check_and_fill_for_cat(ref))
return -1;
/* Fill in specials first */
diff --git a/ref-filter.h b/ref-filter.h
index 5c6e019998716..69271e8c39f40 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -125,6 +125,7 @@ struct ref_format {
* hopefully would be reduced later.
*/
struct expand_data *cat_file_data;
+ int is_cat;
};
#define REF_FORMAT_INIT { NULL, 0, -1 }
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* Re: [PATCH RFC v2 14/25] ref-filter: add is_cat flag
2018-02-05 11:27 ` [PATCH RFC v2 14/25] ref-filter: add is_cat flag Olga Telezhnaya
@ 2018-02-05 21:19 ` Junio C Hamano
0 siblings, 0 replies; 115+ messages in thread
From: Junio C Hamano @ 2018-02-05 21:19 UTC (permalink / raw)
To: Olga Telezhnaya; +Cc: git
Olga Telezhnaya <olyatelezhnaya@gmail.com> writes:
> diff --git a/ref-filter.c b/ref-filter.c
> index 34a54db168265..91290b62450b3 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -101,6 +101,7 @@ static struct used_atom {
> } *used_atom;
> static int used_atom_cnt, need_tagged, need_symref;
> struct expand_data *cat_file_info;
> +static int is_cat = 0;
> ...
> @@ -739,6 +740,7 @@ int verify_ref_format(struct ref_format *format)
> const char *cp, *sp;
>
> cat_file_info = format->cat_file_data;
> + is_cat = format->is_cat;
Eek. The global cat_file_info itself may already be bad enough, but
now we have another one?
Hopefully these are all cleaned up in later steps in the series?
> format->need_color_reset_at_eol = 0;
> for (cp = format->format; *cp && (sp = find_next(cp)); ) {
> const char *color, *ep = strchr(sp, ')');
> @@ -748,7 +750,7 @@ int verify_ref_format(struct ref_format *format)
> return error(_("malformed format string %s"), sp);
> /* sp points at "%(" and ep points at the closing ")" */
>
> - if (format->cat_file_data)
> + if (is_cat)
> at = parse_ref_filter_atom(format, valid_cat_file_atom,
> ARRAY_SIZE(valid_cat_file_atom), sp + 2, ep);
> else {
> @@ -1438,7 +1440,7 @@ int populate_value(struct ref_array_item *ref)
> ref->symref = "";
> }
>
> - if (cat_file_info && check_and_fill_for_cat(ref))
> + if (is_cat && check_and_fill_for_cat(ref))
> return -1;
>
> /* Fill in specials first */
> diff --git a/ref-filter.h b/ref-filter.h
> index 5c6e019998716..69271e8c39f40 100644
> --- a/ref-filter.h
> +++ b/ref-filter.h
> @@ -125,6 +125,7 @@ struct ref_format {
> * hopefully would be reduced later.
> */
> struct expand_data *cat_file_data;
> + int is_cat;
> };
>
> #define REF_FORMAT_INIT { NULL, 0, -1 }
>
> --
> https://github.com/git/git/pull/452
^ permalink raw reply [flat|nested] 115+ messages in thread
* [PATCH RFC v2 05/25] cat-file: move struct expand_data into ref-filter
2018-02-05 11:27 ` [PATCH RFC v2 01/25] " Olga Telezhnaya
` (19 preceding siblings ...)
2018-02-05 11:27 ` [PATCH RFC v2 14/25] ref-filter: add is_cat flag Olga Telezhnaya
@ 2018-02-05 11:27 ` Olga Telezhnaya
2018-02-05 11:27 ` [PATCH RFC v2 11/25] ref-filter: rename field in ref_array_item stuct Olga Telezhnaya
` (3 subsequent siblings)
24 siblings, 0 replies; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-05 11:27 UTC (permalink / raw)
To: git
Need that for further reusing of formatting logic in cat-file.
Have plans to get rid of using expand_data in cat-file at all,
and use it only in ref-filter for collecting, formatting and printing
needed data.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
builtin/cat-file.c | 36 ------------------------------------
ref-filter.h | 36 ++++++++++++++++++++++++++++++++++++
2 files changed, 36 insertions(+), 36 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 98fc5ec069a49..37d6096d201b5 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -176,42 +176,6 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
return 0;
}
-struct expand_data {
- struct object_id oid;
- enum object_type type;
- unsigned long size;
- off_t disk_size;
- const char *rest;
- struct object_id delta_base_oid;
-
- /*
- * If mark_query is true, we do not expand anything, but rather
- * just mark the object_info with items we wish to query.
- */
- int mark_query;
-
- /*
- * Whether to split the input on whitespace before feeding it to
- * get_sha1; this is decided during the mark_query phase based on
- * whether we have a %(rest) token in our format.
- */
- int split_on_whitespace;
-
- /*
- * After a mark_query run, this object_info is set up to be
- * passed to sha1_object_info_extended. It will point to the data
- * elements above, so you can retrieve the response from there.
- */
- struct object_info info;
-
- /*
- * This flag will be true if the requested batch format and options
- * don't require us to call sha1_object_info, which can then be
- * optimized out.
- */
- unsigned skip_object_info : 1;
-};
-
static int is_atom(const char *atom, const char *s, int slen)
{
int alen = strlen(atom);
diff --git a/ref-filter.h b/ref-filter.h
index b75c8ac45248e..17f2ac24d2739 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -72,6 +72,42 @@ struct ref_filter {
verbose;
};
+struct expand_data {
+ struct object_id oid;
+ enum object_type type;
+ unsigned long size;
+ off_t disk_size;
+ const char *rest;
+ struct object_id delta_base_oid;
+
+ /*
+ * If mark_query is true, we do not expand anything, but rather
+ * just mark the object_info with items we wish to query.
+ */
+ int mark_query;
+
+ /*
+ * Whether to split the input on whitespace before feeding it to
+ * get_sha1; this is decided during the mark_query phase based on
+ * whether we have a %(rest) token in our format.
+ */
+ int split_on_whitespace;
+
+ /*
+ * After a mark_query run, this object_info is set up to be
+ * passed to sha1_object_info_extended. It will point to the data
+ * elements above, so you can retrieve the response from there.
+ */
+ struct object_info info;
+
+ /*
+ * This flag will be true if the requested batch format and options
+ * don't require us to call sha1_object_info, which can then be
+ * optimized out.
+ */
+ unsigned skip_object_info : 1;
+};
+
struct ref_format {
/*
* Set these to define the format; make sure you call
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* [PATCH RFC v2 11/25] ref-filter: rename field in ref_array_item stuct
2018-02-05 11:27 ` [PATCH RFC v2 01/25] " Olga Telezhnaya
` (20 preceding siblings ...)
2018-02-05 11:27 ` [PATCH RFC v2 05/25] cat-file: move struct expand_data into ref-filter Olga Telezhnaya
@ 2018-02-05 11:27 ` Olga Telezhnaya
2018-02-05 11:27 ` [PATCH RFC v2 06/25] cat-file: split expand_atom() into 2 functions Olga Telezhnaya
` (2 subsequent siblings)
24 siblings, 0 replies; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-05 11:27 UTC (permalink / raw)
To: git
Rename objectname field to oid in struct ref_array_item.
Next commit will add objectname field that will contain
string representation of object id.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
builtin/cat-file.c | 4 ++--
ref-filter.c | 10 +++++-----
ref-filter.h | 2 +-
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 61b7acc79155d..367b1bd5802dc 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -186,7 +186,7 @@ static void expand_atom(struct strbuf *sb, const char *atom, int len,
struct ref_array_item *item)
{
if (is_atom("objectname", atom, len))
- strbuf_addstr(sb, oid_to_hex(&item->objectname));
+ strbuf_addstr(sb, oid_to_hex(&item->oid));
else if (is_atom("objecttype", atom, len))
strbuf_addstr(sb, typename(item->type));
else if (is_atom("objectsize", atom, len))
@@ -294,7 +294,7 @@ static void batch_object_write(const char *obj_name, struct batch_options *opt,
return;
}
- item.objectname = data->oid;
+ item.oid = data->oid;
item.type = data->type;
item.size = data->size;
item.disk_size = data->disk_size;
diff --git a/ref-filter.c b/ref-filter.c
index 4acd391b5dfac..d09ec1bde6d54 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1489,7 +1489,7 @@ int populate_value(struct ref_array_item *ref)
v->s = xstrdup(buf + 1);
}
continue;
- } else if (!deref && grab_objectname(name, ref->objectname.hash, v, atom)) {
+ } else if (!deref && grab_objectname(name, ref->oid.hash, v, atom)) {
continue;
} else if (!strcmp(name, "HEAD")) {
if (atom->u.head && !strcmp(ref->refname, atom->u.head))
@@ -1534,13 +1534,13 @@ int populate_value(struct ref_array_item *ref)
if (used_atom_cnt <= i)
return 0;
- buf = get_obj(&ref->objectname, &obj, &size, &eaten);
+ buf = get_obj(&ref->oid, &obj, &size, &eaten);
if (!buf)
die(_("missing object %s for %s"),
- oid_to_hex(&ref->objectname), ref->refname);
+ oid_to_hex(&ref->oid), ref->refname);
if (!obj)
die(_("parse_object_buffer failed on %s for %s"),
- oid_to_hex(&ref->objectname), ref->refname);
+ oid_to_hex(&ref->oid), ref->refname);
grab_values(ref->value, 0, obj, buf, size);
if (!eaten)
@@ -1890,7 +1890,7 @@ static struct ref_array_item *new_ref_array_item(const char *refname,
{
struct ref_array_item *ref;
FLEX_ALLOC_STR(ref, refname, refname);
- hashcpy(ref->objectname.hash, objectname);
+ hashcpy(ref->oid.hash, objectname);
ref->flag = flag;
return ref;
diff --git a/ref-filter.h b/ref-filter.h
index e16ea2a990119..87b026b8b76d0 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -34,7 +34,7 @@ struct ref_sorting {
};
struct ref_array_item {
- struct object_id objectname;
+ struct object_id oid;
int flag;
unsigned int kind;
const char *symref;
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* [PATCH RFC v2 06/25] cat-file: split expand_atom() into 2 functions
2018-02-05 11:27 ` [PATCH RFC v2 01/25] " Olga Telezhnaya
` (21 preceding siblings ...)
2018-02-05 11:27 ` [PATCH RFC v2 11/25] ref-filter: rename field in ref_array_item stuct Olga Telezhnaya
@ 2018-02-05 11:27 ` Olga Telezhnaya
2018-02-05 11:27 ` [PATCH RFC v2 25/25] cat-file: update of docs Olga Telezhnaya
2018-02-12 8:08 ` [PATCH v3 01/23] ref-filter: get rid of goto Olga Telezhnaya
24 siblings, 0 replies; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-05 11:27 UTC (permalink / raw)
To: git
Split expand_atom() into 2 different functions,
mark_atom_in_object_info() prepares variable for further filling,
(new) expand_atom() creates resulting string.
Need that for further reusing of formatting logic from ref-filter.
Both functions will be step-by-step removed by the end of this patch.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
builtin/cat-file.c | 73 ++++++++++++++++++++++++++++--------------------------
1 file changed, 38 insertions(+), 35 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 37d6096d201b5..edb04a96d9bd3 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -182,47 +182,47 @@ static int is_atom(const char *atom, const char *s, int slen)
return alen == slen && !memcmp(atom, s, alen);
}
-static void expand_atom(struct strbuf *sb, const char *atom, int len,
- void *vdata)
+static void mark_atom_in_object_info(const char *atom, int len,
+ struct expand_data *data)
{
- struct expand_data *data = vdata;
+ if (is_atom("objectname", atom, len))
+ ; /* do nothing */
+ else if (is_atom("objecttype", atom, len))
+ data->info.typep = &data->type;
+ else if (is_atom("objectsize", atom, len))
+ data->info.sizep = &data->size;
+ else if (is_atom("objectsize:disk", atom, len))
+ data->info.disk_sizep = &data->disk_size;
+ else if (is_atom("rest", atom, len))
+ data->split_on_whitespace = 1;
+ else if (is_atom("deltabase", atom, len))
+ data->info.delta_base_sha1 = data->delta_base_oid.hash;
+ else
+ die("unknown format element: %.*s", len, atom);
+}
- if (is_atom("objectname", atom, len)) {
- if (!data->mark_query)
- strbuf_addstr(sb, oid_to_hex(&data->oid));
- } else if (is_atom("objecttype", atom, len)) {
- if (data->mark_query)
- data->info.typep = &data->type;
- else
- strbuf_addstr(sb, typename(data->type));
- } else if (is_atom("objectsize", atom, len)) {
- if (data->mark_query)
- data->info.sizep = &data->size;
- else
- strbuf_addf(sb, "%lu", data->size);
- } else if (is_atom("objectsize:disk", atom, len)) {
- if (data->mark_query)
- data->info.disk_sizep = &data->disk_size;
- else
- strbuf_addf(sb, "%"PRIuMAX, (uintmax_t)data->disk_size);
- } else if (is_atom("rest", atom, len)) {
- if (data->mark_query)
- data->split_on_whitespace = 1;
- else if (data->rest)
+static void expand_atom(struct strbuf *sb, const char *atom, int len,
+ struct expand_data *data)
+{
+ if (is_atom("objectname", atom, len))
+ strbuf_addstr(sb, oid_to_hex(&data->oid));
+ else if (is_atom("objecttype", atom, len))
+ strbuf_addstr(sb, typename(data->type));
+ else if (is_atom("objectsize", atom, len))
+ strbuf_addf(sb, "%lu", data->size);
+ else if (is_atom("objectsize:disk", atom, len))
+ strbuf_addf(sb, "%"PRIuMAX, (uintmax_t)data->disk_size);
+ else if (is_atom("rest", atom, len)) {
+ if (data->rest)
strbuf_addstr(sb, data->rest);
- } else if (is_atom("deltabase", atom, len)) {
- if (data->mark_query)
- data->info.delta_base_sha1 = data->delta_base_oid.hash;
- else
- strbuf_addstr(sb,
- oid_to_hex(&data->delta_base_oid));
- } else
- die("unknown format element: %.*s", len, atom);
+ } else if (is_atom("deltabase", atom, len))
+ strbuf_addstr(sb, oid_to_hex(&data->delta_base_oid));
}
-static size_t expand_format(struct strbuf *sb, const char *start, void *data)
+static size_t expand_format(struct strbuf *sb, const char *start, void *vdata)
{
const char *end;
+ struct expand_data *data = vdata;
if (*start != '(')
return 0;
@@ -230,7 +230,10 @@ static size_t expand_format(struct strbuf *sb, const char *start, void *data)
if (!end)
die("format element '%s' does not end in ')'", start);
- expand_atom(sb, start + 1, end - start - 1, data);
+ if (data->mark_query)
+ mark_atom_in_object_info(start + 1, end - start - 1, data);
+ else
+ expand_atom(sb, start + 1, end - start - 1, data);
return end - start + 1;
}
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* [PATCH RFC v2 25/25] cat-file: update of docs
2018-02-05 11:27 ` [PATCH RFC v2 01/25] " Olga Telezhnaya
` (22 preceding siblings ...)
2018-02-05 11:27 ` [PATCH RFC v2 06/25] cat-file: split expand_atom() into 2 functions Olga Telezhnaya
@ 2018-02-05 11:27 ` Olga Telezhnaya
2018-02-12 8:08 ` [PATCH v3 01/23] ref-filter: get rid of goto Olga Telezhnaya
24 siblings, 0 replies; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-05 11:27 UTC (permalink / raw)
To: git
Update the docs for cat-file command. Some new formatting atoms added
because of reusing ref-filter code.
We do not support cat-file atoms in general formatting logic
(there is just the support for cat-file), that is why some of the atoms
are still explained in cat-file docs.
We need to move these explanations when atoms will be supported
by other commands.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
Documentation/git-cat-file.txt | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index f90f09b03fae5..90639ac21d0e8 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -187,17 +187,8 @@ linkgit:git-rev-parse[1].
You can specify the information shown for each object by using a custom
`<format>`. The `<format>` is copied literally to stdout for each
object, with placeholders of the form `%(atom)` expanded, followed by a
-newline. The available atoms are:
-
-`objectname`::
- The 40-hex object name of the object.
-
-`objecttype`::
- The type of the object (the same as `cat-file -t` reports).
-
-`objectsize`::
- The size, in bytes, of the object (the same as `cat-file -s`
- reports).
+newline. The available atoms are the same as that of
+linkgit:git-for-each-ref[1], but there are some additional ones:
`objectsize:disk`::
The size, in bytes, that the object takes up on disk. See the
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* [PATCH v3 01/23] ref-filter: get rid of goto
2018-02-05 11:27 ` [PATCH RFC v2 01/25] " Olga Telezhnaya
` (23 preceding siblings ...)
2018-02-05 11:27 ` [PATCH RFC v2 25/25] cat-file: update of docs Olga Telezhnaya
@ 2018-02-12 8:08 ` Olga Telezhnaya
2018-02-12 8:08 ` [PATCH v3 06/23] cat-file: split expand_atom() into 2 functions Olga Telezhnaya
` (22 more replies)
24 siblings, 23 replies; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-12 8:08 UTC (permalink / raw)
To: git
Get rid of goto command in ref-filter for better readability.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
ref-filter.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index f9e25aea7a97e..d04295e33448e 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1477,12 +1477,13 @@ static void populate_value(struct ref_array_item *ref)
for (i = 0; i < used_atom_cnt; i++) {
struct atom_value *v = &ref->value[i];
- if (v->s == NULL)
- goto need_obj;
+ if (v->s == NULL) {
+ break;
+ }
}
- return;
+ if (used_atom_cnt <= i)
+ return;
- need_obj:
buf = get_obj(&ref->objectname, &obj, &size, &eaten);
if (!buf)
die(_("missing object %s for %s"),
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* [PATCH v3 06/23] cat-file: split expand_atom() into 2 functions
2018-02-12 8:08 ` [PATCH v3 01/23] ref-filter: get rid of goto Olga Telezhnaya
@ 2018-02-12 8:08 ` Olga Telezhnaya
2018-02-15 5:28 ` Jeff King
2018-02-12 8:08 ` [PATCH v3 18/23] cat-file: reuse printing logic from ref-filter Olga Telezhnaya
` (21 subsequent siblings)
22 siblings, 1 reply; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-12 8:08 UTC (permalink / raw)
To: git
Split expand_atom() into 2 different functions,
mark_atom_in_object_info() prepares variable for further filling,
(new) expand_atom() creates resulting string.
Need that for further reusing of formatting logic from ref-filter.
Both functions will be step-by-step removed by the end of this patch.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
builtin/cat-file.c | 73 ++++++++++++++++++++++++++++--------------------------
1 file changed, 38 insertions(+), 35 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 37d6096d201b5..edb04a96d9bd3 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -182,47 +182,47 @@ static int is_atom(const char *atom, const char *s, int slen)
return alen == slen && !memcmp(atom, s, alen);
}
-static void expand_atom(struct strbuf *sb, const char *atom, int len,
- void *vdata)
+static void mark_atom_in_object_info(const char *atom, int len,
+ struct expand_data *data)
{
- struct expand_data *data = vdata;
+ if (is_atom("objectname", atom, len))
+ ; /* do nothing */
+ else if (is_atom("objecttype", atom, len))
+ data->info.typep = &data->type;
+ else if (is_atom("objectsize", atom, len))
+ data->info.sizep = &data->size;
+ else if (is_atom("objectsize:disk", atom, len))
+ data->info.disk_sizep = &data->disk_size;
+ else if (is_atom("rest", atom, len))
+ data->split_on_whitespace = 1;
+ else if (is_atom("deltabase", atom, len))
+ data->info.delta_base_sha1 = data->delta_base_oid.hash;
+ else
+ die("unknown format element: %.*s", len, atom);
+}
- if (is_atom("objectname", atom, len)) {
- if (!data->mark_query)
- strbuf_addstr(sb, oid_to_hex(&data->oid));
- } else if (is_atom("objecttype", atom, len)) {
- if (data->mark_query)
- data->info.typep = &data->type;
- else
- strbuf_addstr(sb, typename(data->type));
- } else if (is_atom("objectsize", atom, len)) {
- if (data->mark_query)
- data->info.sizep = &data->size;
- else
- strbuf_addf(sb, "%lu", data->size);
- } else if (is_atom("objectsize:disk", atom, len)) {
- if (data->mark_query)
- data->info.disk_sizep = &data->disk_size;
- else
- strbuf_addf(sb, "%"PRIuMAX, (uintmax_t)data->disk_size);
- } else if (is_atom("rest", atom, len)) {
- if (data->mark_query)
- data->split_on_whitespace = 1;
- else if (data->rest)
+static void expand_atom(struct strbuf *sb, const char *atom, int len,
+ struct expand_data *data)
+{
+ if (is_atom("objectname", atom, len))
+ strbuf_addstr(sb, oid_to_hex(&data->oid));
+ else if (is_atom("objecttype", atom, len))
+ strbuf_addstr(sb, typename(data->type));
+ else if (is_atom("objectsize", atom, len))
+ strbuf_addf(sb, "%lu", data->size);
+ else if (is_atom("objectsize:disk", atom, len))
+ strbuf_addf(sb, "%"PRIuMAX, (uintmax_t)data->disk_size);
+ else if (is_atom("rest", atom, len)) {
+ if (data->rest)
strbuf_addstr(sb, data->rest);
- } else if (is_atom("deltabase", atom, len)) {
- if (data->mark_query)
- data->info.delta_base_sha1 = data->delta_base_oid.hash;
- else
- strbuf_addstr(sb,
- oid_to_hex(&data->delta_base_oid));
- } else
- die("unknown format element: %.*s", len, atom);
+ } else if (is_atom("deltabase", atom, len))
+ strbuf_addstr(sb, oid_to_hex(&data->delta_base_oid));
}
-static size_t expand_format(struct strbuf *sb, const char *start, void *data)
+static size_t expand_format(struct strbuf *sb, const char *start, void *vdata)
{
const char *end;
+ struct expand_data *data = vdata;
if (*start != '(')
return 0;
@@ -230,7 +230,10 @@ static size_t expand_format(struct strbuf *sb, const char *start, void *data)
if (!end)
die("format element '%s' does not end in ')'", start);
- expand_atom(sb, start + 1, end - start - 1, data);
+ if (data->mark_query)
+ mark_atom_in_object_info(start + 1, end - start - 1, data);
+ else
+ expand_atom(sb, start + 1, end - start - 1, data);
return end - start + 1;
}
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* Re: [PATCH v3 06/23] cat-file: split expand_atom() into 2 functions
2018-02-12 8:08 ` [PATCH v3 06/23] cat-file: split expand_atom() into 2 functions Olga Telezhnaya
@ 2018-02-15 5:28 ` Jeff King
0 siblings, 0 replies; 115+ messages in thread
From: Jeff King @ 2018-02-15 5:28 UTC (permalink / raw)
To: Olga Telezhnaya; +Cc: git
On Mon, Feb 12, 2018 at 08:08:54AM +0000, Olga Telezhnaya wrote:
> Split expand_atom() into 2 different functions,
> mark_atom_in_object_info() prepares variable for further filling,
> (new) expand_atom() creates resulting string.
> Need that for further reusing of formatting logic from ref-filter.
> Both functions will be step-by-step removed by the end of this patch.
Hmm, this is duplicating the list of atoms. One of the reasons for the
mark_query flag in the original is to have a single list, so that it
can't go out of sync. So I'd hope to see this split go away by the end
of the series...
-Peff
^ permalink raw reply [flat|nested] 115+ messages in thread
* [PATCH v3 18/23] cat-file: reuse printing logic from ref-filter
2018-02-12 8:08 ` [PATCH v3 01/23] ref-filter: get rid of goto Olga Telezhnaya
2018-02-12 8:08 ` [PATCH v3 06/23] cat-file: split expand_atom() into 2 functions Olga Telezhnaya
@ 2018-02-12 8:08 ` Olga Telezhnaya
2018-02-12 8:08 ` [PATCH v3 17/23] ref-filter: make valid_atom general again Olga Telezhnaya
` (20 subsequent siblings)
22 siblings, 0 replies; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-12 8:08 UTC (permalink / raw)
To: git
Reuse code from ref-filter to print resulting message.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
builtin/cat-file.c | 51 ++++-----------------------------------------------
ref-filter.c | 21 +++++++++++++++++++--
2 files changed, 23 insertions(+), 49 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 273b4038e3893..21007995c5ac6 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -176,45 +176,6 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
return 0;
}
-static int is_atom(const char *atom, const char *s, int slen)
-{
- int alen = strlen(atom);
- return alen == slen && !memcmp(atom, s, alen);
-}
-
-static void expand_atom(struct strbuf *sb, const char *atom, int len,
- struct ref_array_item *item)
-{
- if (is_atom("objectname", atom, len))
- strbuf_addstr(sb, oid_to_hex(&item->oid));
- else if (is_atom("objecttype", atom, len))
- strbuf_addstr(sb, typename(item->type));
- else if (is_atom("objectsize", atom, len))
- strbuf_addf(sb, "%lu", item->size);
- else if (is_atom("objectsize:disk", atom, len))
- strbuf_addf(sb, "%"PRIuMAX, (uintmax_t)item->disk_size);
- else if (is_atom("rest", atom, len)) {
- if (item->rest)
- strbuf_addstr(sb, item->rest);
- } else if (is_atom("deltabase", atom, len))
- strbuf_addstr(sb, oid_to_hex(item->delta_base_oid));
-}
-
-static size_t expand_format(struct strbuf *sb, const char *start, void *data)
-{
- const char *end;
- struct ref_array_item *item = data;
-
- if (*start != '(')
- return 0;
- end = strchr(start + 1, ')');
- if (!end)
- die("format element '%s' does not end in ')'", start);
-
- expand_atom(sb, start + 1, end - start - 1, item);
- return end - start + 1;
-}
-
static void batch_write(struct batch_options *opt, const void *data, int len)
{
if (opt->buffer_output) {
@@ -282,23 +243,19 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d
static void batch_object_write(const char *obj_name, struct batch_options *opt,
struct expand_data *data)
{
- struct strbuf buf = STRBUF_INIT;
struct ref_array_item item = {0};
item.oid = data->oid;
item.rest = data->rest;
item.objectname = obj_name;
- if (populate_value(&item))
+ if (show_ref_array_item(&item, &opt->format))
return;
-
- data->type = item.type;
- strbuf_expand(&buf, opt->format.format, expand_format, &item);
- strbuf_addch(&buf, '\n');
- batch_write(opt, buf.buf, buf.len);
- strbuf_release(&buf);
+ if (!opt->buffer_output)
+ fflush(stdout);
if (opt->print_contents) {
+ data->type = item.type;
print_object_or_die(opt, data);
batch_write(opt, "\n", 1);
}
diff --git a/ref-filter.c b/ref-filter.c
index ee311d51ff81c..eb53b0babdb83 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1465,7 +1465,21 @@ int populate_value(struct ref_array_item *ref)
name++;
}
- if (starts_with(name, "refname"))
+ if (cat_file_info.is_cat_file) {
+ if (starts_with(name, "objectname"))
+ v->s = oid_to_hex(&ref->oid);
+ else if (starts_with(name, "objecttype"))
+ v->s = typename(ref->type);
+ else if (starts_with(name, "objectsize")) {
+ v->s = xstrfmt("%lu", ref->size);
+ } else if (starts_with(name, "objectsize:disk")) {
+ v->s = xstrfmt("%"PRIuMAX, (uintmax_t)ref->disk_size);
+ } else if (starts_with(name, "rest"))
+ v->s = ref->rest;
+ else if (starts_with(name, "deltabase"))
+ v->s = xstrdup(oid_to_hex(ref->delta_base_oid));
+ continue;
+ } else if (starts_with(name, "refname"))
refname = get_refname(atom, ref);
else if (starts_with(name, "symref"))
refname = get_symref(atom, ref);
@@ -2207,6 +2221,7 @@ int format_ref_array_item(struct ref_array_item *info,
{
const char *cp, *sp, *ep;
struct ref_formatting_state state = REF_FORMATTING_STATE_INIT;
+ int retval = 0;
state.quote_style = format->quote_style;
push_stack_element(&state.stack);
@@ -2223,6 +2238,8 @@ int format_ref_array_item(struct ref_array_item *info,
return -1;
atomv->handler(atomv, &state);
}
+ if (cat_file_info.is_cat_file && strlen(format->format) == 0)
+ retval = check_and_fill_for_cat(info);
if (*cp) {
sp = cp + strlen(cp);
append_literal(cp, sp, &state);
@@ -2236,7 +2253,7 @@ int format_ref_array_item(struct ref_array_item *info,
die(_("format: %%(end) atom missing"));
strbuf_addbuf(final_buf, &state.stack->output);
pop_stack_element(&state.stack);
- return 0;
+ return retval;
}
int show_ref_array_item(struct ref_array_item *info,
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* [PATCH v3 17/23] ref-filter: make valid_atom general again
2018-02-12 8:08 ` [PATCH v3 01/23] ref-filter: get rid of goto Olga Telezhnaya
2018-02-12 8:08 ` [PATCH v3 06/23] cat-file: split expand_atom() into 2 functions Olga Telezhnaya
2018-02-12 8:08 ` [PATCH v3 18/23] cat-file: reuse printing logic from ref-filter Olga Telezhnaya
@ 2018-02-12 8:08 ` Olga Telezhnaya
2018-02-15 5:53 ` Jeff King
2018-02-12 8:08 ` [PATCH v3 03/23] cat-file: reuse struct ref_format Olga Telezhnaya
` (19 subsequent siblings)
22 siblings, 1 reply; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-12 8:08 UTC (permalink / raw)
To: git
Stop using valid_cat_file_atom, making the code more general.
Further commits will contain some tests, docs and
support of new features.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
ref-filter.c | 34 +++++++++-------------------------
1 file changed, 9 insertions(+), 25 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index cc70bcf2bb8b1..ee311d51ff81c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -358,8 +358,8 @@ static struct valid_atom {
void (*parser)(const struct ref_format *format, struct used_atom *atom, const char *arg);
} valid_atom[] = {
{ "refname" , FIELD_STR, refname_atom_parser },
- { "objecttype" },
- { "objectsize", FIELD_ULONG },
+ { "objecttype", FIELD_STR, objecttype_atom_parser },
+ { "objectsize", FIELD_ULONG, objectsize_atom_parser },
{ "objectname", FIELD_STR, objectname_atom_parser },
{ "tree" },
{ "parent" },
@@ -396,12 +396,6 @@ static struct valid_atom {
{ "if", FIELD_STR, if_atom_parser },
{ "then" },
{ "else" },
-};
-
-static struct valid_atom valid_cat_file_atom[] = {
- { "objectname" },
- { "objecttype", FIELD_STR, objecttype_atom_parser },
- { "objectsize", FIELD_ULONG, objectsize_atom_parser },
{ "rest" },
{ "deltabase", FIELD_STR, deltabase_atom_parser },
};
@@ -431,7 +425,6 @@ struct atom_value {
* Used to parse format string and sort specifiers
*/
static int parse_ref_filter_atom(const struct ref_format *format,
- const struct valid_atom *valid_atom, int n_atoms,
const char *atom, const char *ep)
{
const char *sp;
@@ -461,13 +454,13 @@ static int parse_ref_filter_atom(const struct ref_format *format,
atom_len = (arg ? arg : ep) - sp;
/* Is the atom a valid one? */
- for (i = 0; i < n_atoms; i++) {
+ for (i = 0; i < ARRAY_SIZE(valid_atom); i++) {
int len = strlen(valid_atom[i].name);
if (len == atom_len && !memcmp(valid_atom[i].name, sp, len))
break;
}
- if (n_atoms <= i)
+ if (ARRAY_SIZE(valid_atom) <= i)
die(_("unknown field name: %.*s"), (int)(ep-atom), atom);
/* Add it in, including the deref prefix */
@@ -761,15 +754,9 @@ int verify_ref_format(struct ref_format *format)
return error(_("malformed format string %s"), sp);
/* sp points at "%(" and ep points at the closing ")" */
- if (cat_file_info.is_cat_file)
- at = parse_ref_filter_atom(format, valid_cat_file_atom,
- ARRAY_SIZE(valid_cat_file_atom), sp + 2, ep);
- else {
- at = parse_ref_filter_atom(format, valid_atom,
- ARRAY_SIZE(valid_atom), sp + 2, ep);
- if (skip_prefix(used_atom[at].name, "color:", &color))
- format->need_color_reset_at_eol = !!strcmp(color, "reset");
- }
+ at = parse_ref_filter_atom(format, sp + 2, ep);
+ if (skip_prefix(used_atom[at].name, "color:", &color))
+ format->need_color_reset_at_eol = !!strcmp(color, "reset");
cp = ep + 1;
}
@@ -2231,9 +2218,7 @@ int format_ref_array_item(struct ref_array_item *info,
if (cp < sp)
append_literal(cp, sp, &state);
if (get_ref_atom_value(info,
- parse_ref_filter_atom(format, valid_atom,
- ARRAY_SIZE(valid_atom),
- sp + 2, ep),
+ parse_ref_filter_atom(format, sp + 2, ep),
&atomv))
return -1;
atomv->handler(atomv, &state);
@@ -2286,8 +2271,7 @@ static int parse_sorting_atom(const char *atom)
*/
struct ref_format dummy = REF_FORMAT_INIT;
const char *end = atom + strlen(atom);
- return parse_ref_filter_atom(&dummy, valid_atom,
- ARRAY_SIZE(valid_atom), atom, end);
+ return parse_ref_filter_atom(&dummy, atom, end);
}
/* If no sorting option is given, use refname to sort as default */
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* [PATCH v3 03/23] cat-file: reuse struct ref_format
2018-02-12 8:08 ` [PATCH v3 01/23] ref-filter: get rid of goto Olga Telezhnaya
` (2 preceding siblings ...)
2018-02-12 8:08 ` [PATCH v3 17/23] ref-filter: make valid_atom general again Olga Telezhnaya
@ 2018-02-12 8:08 ` Olga Telezhnaya
2018-02-15 5:18 ` Jeff King
2018-02-12 8:08 ` [PATCH v3 23/23] cat-file: update of docs Olga Telezhnaya
` (18 subsequent siblings)
22 siblings, 1 reply; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-12 8:08 UTC (permalink / raw)
To: git
Start using ref_format struct instead of simple char*.
Need that for further reusing of formatting logic from ref-filter.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
builtin/cat-file.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index f5fa4fd75af26..98fc5ec069a49 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -13,15 +13,16 @@
#include "tree-walk.h"
#include "sha1-array.h"
#include "packfile.h"
+#include "ref-filter.h"
struct batch_options {
+ struct ref_format format;
int enabled;
int follow_symlinks;
int print_contents;
int buffer_output;
int all_objects;
int cmdmode; /* may be 'w' or 'c' for --filters or --textconv */
- const char *format;
};
static const char *force_path;
@@ -348,7 +349,7 @@ static void batch_object_write(const char *obj_name, struct batch_options *opt,
return;
}
- strbuf_expand(&buf, opt->format, expand_format, data);
+ strbuf_expand(&buf, opt->format.format, expand_format, data);
strbuf_addch(&buf, '\n');
batch_write(opt, buf.buf, buf.len);
strbuf_release(&buf);
@@ -441,8 +442,8 @@ static int batch_objects(struct batch_options *opt)
int save_warning;
int retval = 0;
- if (!opt->format)
- opt->format = "%(objectname) %(objecttype) %(objectsize)";
+ if (!opt->format.format)
+ opt->format.format = "%(objectname) %(objecttype) %(objectsize)";
/*
* Expand once with our special mark_query flag, which will prime the
@@ -451,7 +452,7 @@ static int batch_objects(struct batch_options *opt)
*/
memset(&data, 0, sizeof(data));
data.mark_query = 1;
- strbuf_expand(&buf, opt->format, expand_format, &data);
+ strbuf_expand(&buf, opt->format.format, expand_format, &data);
data.mark_query = 0;
if (opt->cmdmode)
data.split_on_whitespace = 1;
@@ -543,7 +544,7 @@ static int batch_option_callback(const struct option *opt,
bo->enabled = 1;
bo->print_contents = !strcmp(opt->long_name, "batch");
- bo->format = arg;
+ bo->format.format = arg;
return 0;
}
@@ -552,7 +553,7 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
{
int opt = 0;
const char *exp_type = NULL, *obj_name = NULL;
- struct batch_options batch = {0};
+ struct batch_options batch = { REF_FORMAT_INIT };
int unknown_type = 0;
const struct option options[] = {
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* [PATCH v3 23/23] cat-file: update of docs
2018-02-12 8:08 ` [PATCH v3 01/23] ref-filter: get rid of goto Olga Telezhnaya
` (3 preceding siblings ...)
2018-02-12 8:08 ` [PATCH v3 03/23] cat-file: reuse struct ref_format Olga Telezhnaya
@ 2018-02-12 8:08 ` Olga Telezhnaya
2018-02-15 6:00 ` Jeff King
2018-02-12 8:08 ` [PATCH v3 12/23] cat-file: start reusing populate_value() Olga Telezhnaya
` (17 subsequent siblings)
22 siblings, 1 reply; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-12 8:08 UTC (permalink / raw)
To: git
Update the docs for cat-file command. Some new formatting atoms added
because of reusing ref-filter code.
We do not support cat-file atoms in general formatting logic
(there is just the support for cat-file), that is why some of the atoms
are still explained in cat-file docs.
We need to move these explanations when atoms will be supported
by other commands.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
Documentation/git-cat-file.txt | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index f90f09b03fae5..90639ac21d0e8 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -187,17 +187,8 @@ linkgit:git-rev-parse[1].
You can specify the information shown for each object by using a custom
`<format>`. The `<format>` is copied literally to stdout for each
object, with placeholders of the form `%(atom)` expanded, followed by a
-newline. The available atoms are:
-
-`objectname`::
- The 40-hex object name of the object.
-
-`objecttype`::
- The type of the object (the same as `cat-file -t` reports).
-
-`objectsize`::
- The size, in bytes, of the object (the same as `cat-file -s`
- reports).
+newline. The available atoms are the same as that of
+linkgit:git-for-each-ref[1], but there are some additional ones:
`objectsize:disk`::
The size, in bytes, that the object takes up on disk. See the
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* Re: [PATCH v3 23/23] cat-file: update of docs
2018-02-12 8:08 ` [PATCH v3 23/23] cat-file: update of docs Olga Telezhnaya
@ 2018-02-15 6:00 ` Jeff King
0 siblings, 0 replies; 115+ messages in thread
From: Jeff King @ 2018-02-15 6:00 UTC (permalink / raw)
To: Olga Telezhnaya; +Cc: git
On Mon, Feb 12, 2018 at 08:08:54AM +0000, Olga Telezhnaya wrote:
> Update the docs for cat-file command. Some new formatting atoms added
> because of reusing ref-filter code.
> We do not support cat-file atoms in general formatting logic
> (there is just the support for cat-file), that is why some of the atoms
> are still explained in cat-file docs.
> We need to move these explanations when atoms will be supported
> by other commands.
OK, so I've read through the whole series now. I still think it really
needs some squashing in the middle to turn into a more comprehensible
series.
And I think by the end we need to address the atoms here that don't work
in ref-filter. We certainly want them to work in the long term, and the
fact that they don't is I think pointing to having the wrong
architecture here in the intermediate step. We should just have a single
set of placeholders, with no special "cat_file_data". If I understand
correctly, that's what's causing those atoms not to work in
for-each-ref.
-Peff
^ permalink raw reply [flat|nested] 115+ messages in thread
* [PATCH v3 12/23] cat-file: start reusing populate_value()
2018-02-12 8:08 ` [PATCH v3 01/23] ref-filter: get rid of goto Olga Telezhnaya
` (4 preceding siblings ...)
2018-02-12 8:08 ` [PATCH v3 23/23] cat-file: update of docs Olga Telezhnaya
@ 2018-02-12 8:08 ` Olga Telezhnaya
2018-02-15 5:42 ` Jeff King
2018-02-12 8:08 ` [PATCH v3 10/23] ref-filter: make populate_value() global Olga Telezhnaya
` (16 subsequent siblings)
22 siblings, 1 reply; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-12 8:08 UTC (permalink / raw)
To: git
Move logic related to getting object info from cat-file to ref-filter.
It will help to reuse whole formatting logic from ref-filter further.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
builtin/cat-file.c | 17 ++++-------------
ref-filter.c | 20 ++++++++++++++++++++
ref-filter.h | 1 +
3 files changed, 25 insertions(+), 13 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 0c362828ad81e..6db57e3533806 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -285,21 +285,12 @@ static void batch_object_write(const char *obj_name, struct batch_options *opt,
struct strbuf buf = STRBUF_INIT;
struct ref_array_item item = {0};
- if (!data->skip_object_info &&
- sha1_object_info_extended(data->oid.hash, &data->info,
- OBJECT_INFO_LOOKUP_REPLACE) < 0) {
- printf("%s missing\n",
- obj_name ? obj_name : oid_to_hex(&data->oid));
- fflush(stdout);
- return;
- }
-
item.oid = data->oid;
- item.type = data->type;
- item.size = data->size;
- item.disk_size = data->disk_size;
item.rest = data->rest;
- item.delta_base_oid = &data->delta_base_oid;
+ item.objectname = obj_name;
+
+ if (populate_value(&item))
+ return;
strbuf_expand(&buf, opt->format.format, expand_format, &item);
strbuf_addch(&buf, '\n');
diff --git a/ref-filter.c b/ref-filter.c
index d09ec1bde6d54..3f92a27d98b6c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1403,6 +1403,23 @@ static const char *get_refname(struct used_atom *atom, struct ref_array_item *re
return show_ref(&atom->u.refname, ref->refname);
}
+static int check_and_fill_for_cat(struct ref_array_item *ref)
+{
+ if (!cat_file_info->skip_object_info &&
+ sha1_object_info_extended(ref->oid.hash, &cat_file_info->info,
+ OBJECT_INFO_LOOKUP_REPLACE) < 0) {
+ const char *e = ref->objectname;
+ printf("%s missing\n", e ? e : oid_to_hex(&ref->oid));
+ fflush(stdout);
+ return -1;
+ }
+ ref->type = cat_file_info->type;
+ ref->size = cat_file_info->size;
+ ref->disk_size = cat_file_info->disk_size;
+ ref->delta_base_oid = &cat_file_info->delta_base_oid;
+ return 0;
+}
+
/*
* Parse the object referred by ref, and grab needed value.
* Return 0 if everything was successful, -1 otherwise.
@@ -1424,6 +1441,9 @@ int populate_value(struct ref_array_item *ref)
ref->symref = "";
}
+ if (cat_file_info && check_and_fill_for_cat(ref))
+ return -1;
+
/* Fill in specials first */
for (i = 0; i < used_atom_cnt; i++) {
struct used_atom *atom = &used_atom[i];
diff --git a/ref-filter.h b/ref-filter.h
index 87b026b8b76d0..5c6e019998716 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -45,6 +45,7 @@ struct ref_array_item {
off_t disk_size;
const char *rest;
struct object_id *delta_base_oid;
+ const char *objectname;
char refname[FLEX_ARRAY];
};
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* [PATCH v3 10/23] ref-filter: make populate_value() global
2018-02-12 8:08 ` [PATCH v3 01/23] ref-filter: get rid of goto Olga Telezhnaya
` (5 preceding siblings ...)
2018-02-12 8:08 ` [PATCH v3 12/23] cat-file: start reusing populate_value() Olga Telezhnaya
@ 2018-02-12 8:08 ` Olga Telezhnaya
2018-02-12 8:08 ` [PATCH v3 22/23] cat-file: tests for new atoms added Olga Telezhnaya
` (15 subsequent siblings)
22 siblings, 0 replies; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-12 8:08 UTC (permalink / raw)
To: git
Make function global for further using in cat-file.
In the end of patch series this function becomes internal again,
so this is a part of middle step. cat-file would use more general
functions further.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
ref-filter.c | 2 +-
ref-filter.h | 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/ref-filter.c b/ref-filter.c
index 5c75259b1ab8c..4acd391b5dfac 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1407,7 +1407,7 @@ static const char *get_refname(struct used_atom *atom, struct ref_array_item *re
* Parse the object referred by ref, and grab needed value.
* Return 0 if everything was successful, -1 otherwise.
*/
-static int populate_value(struct ref_array_item *ref)
+int populate_value(struct ref_array_item *ref)
{
void *buf;
struct object *obj;
diff --git a/ref-filter.h b/ref-filter.h
index 781921d4e0978..e16ea2a990119 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -182,4 +182,7 @@ void setup_ref_filter_porcelain_msg(void);
void pretty_print_ref(const char *name, const unsigned char *sha1,
const struct ref_format *format);
+/* Fill the values of request and prepare all data for final string creation */
+int populate_value(struct ref_array_item *ref);
+
#endif /* REF_FILTER_H */
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* [PATCH v3 22/23] cat-file: tests for new atoms added
2018-02-12 8:08 ` [PATCH v3 01/23] ref-filter: get rid of goto Olga Telezhnaya
` (6 preceding siblings ...)
2018-02-12 8:08 ` [PATCH v3 10/23] ref-filter: make populate_value() global Olga Telezhnaya
@ 2018-02-12 8:08 ` Olga Telezhnaya
2018-02-16 14:55 ` Adam Dinwoodie
2018-02-12 8:08 ` [PATCH v3 04/23] ref-filter: make valid_atom as function parameter Olga Telezhnaya
` (14 subsequent siblings)
22 siblings, 1 reply; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-12 8:08 UTC (permalink / raw)
To: git
Add some tests for new formatting atoms from ref-filter.
Some of new atoms are supported automatically,
some of them are expanded into empty string
(because they are useless for some types of objects),
some of them could be supported later in other patches.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
t/t1006-cat-file.sh | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index b19f332694620..e72fcaf0e02c5 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -20,6 +20,19 @@ maybe_remove_timestamp () {
fi
}
+test_atom () {
+ name=$1
+ sha1=$2
+ atoms=$3
+ expected=$4
+
+ test_expect_success "$name" '
+ echo "$expected" >expect &&
+ echo $sha1 | git cat-file --batch-check="$atoms" >actual &&
+ test_cmp expect actual
+ '
+}
+
run_tests () {
type=$1
sha1=$2
@@ -119,6 +132,13 @@ $content"
maybe_remove_timestamp "$(cat actual.full)" $no_ts >actual &&
test_cmp expect actual
'
+
+ for atom in refname parent body trailers upstream push symref flag
+ do
+ test_atom "Check %($atom) gives empty output" "$sha1" "%($atom)" ""
+ done
+
+ test_atom "Check %(HEAD) gives only one space as output" "$sha1" '%(HEAD)' ' '
}
hello_content="Hello World"
@@ -140,6 +160,12 @@ test_expect_success '--batch-check without %(rest) considers whole line' '
test_cmp expect actual
'
+shortname=`echo $hello_sha1 | sed 's/^.\{0\}\(.\{7\}\).*/\1/'`
+test_atom 'Check format option %(objectname:short) works' "$hello_sha1" '%(objectname:short)' "$shortname"
+
+test_atom 'Check format option %(align) is not broken' \
+ "$hello_sha1" "%(align:8)%(objecttype)%(end)%(objectname)" "blob $hello_sha1"
+
tree_sha1=$(git write-tree)
tree_size=33
tree_pretty_content="100644 blob $hello_sha1 hello"
@@ -157,6 +183,17 @@ $commit_message"
run_tests 'commit' $commit_sha1 $commit_size "$commit_content" "$commit_content" 1
+test_atom "Check format option %(if) is not broken" "$commit_sha1" \
+ "%(if)%(author)%(then)%(objectname)%(end)" "$commit_sha1"
+test_atom "Check %(tree) works for commit" "$commit_sha1" "%(tree)" "$tree_sha1"
+test_atom "Check %(numparent) works for commit" "$commit_sha1" "%(numparent)" "0"
+test_atom "Check %(authorname) works for commit" "$commit_sha1" "%(authorname)" "$GIT_AUTHOR_NAME"
+test_atom "Check %(authoremail) works for commit" "$commit_sha1" "%(authoremail)" "<$GIT_AUTHOR_EMAIL>"
+test_atom "Check %(committername) works for commit" "$commit_sha1" "%(committername)" "$GIT_COMMITTER_NAME"
+test_atom "Check %(committeremail) works for commit" "$commit_sha1" "%(committeremail)" "<$GIT_COMMITTER_EMAIL>"
+test_atom "Check %(subject) works for commit" "$commit_sha1" "%(subject)" "$commit_message"
+test_atom "Check %(contents) works for commit" "$commit_sha1" "%(contents)" "$commit_message"
+
tag_header_without_timestamp="object $hello_sha1
type blob
tag hellotag
@@ -171,6 +208,17 @@ tag_size=$(strlen "$tag_content")
run_tests 'tag' $tag_sha1 $tag_size "$tag_content" "$tag_content" 1
+test_atom "Check %(object) works for tag" "$tag_sha1" "%(object)" "$hello_sha1"
+test_atom "Check %(type) works for tag" "$tag_sha1" "%(type)" "blob"
+test_atom "Check %(tag) works for tag" "$tag_sha1" "%(tag)" "hellotag"
+test_atom "Check %(taggername) works for tag" "$tag_sha1" "%(taggername)" "$GIT_COMMITTER_NAME"
+test_atom "Check %(taggeremail) works for tag" "$tag_sha1" "%(taggeremail)" "<$GIT_COMMITTER_EMAIL>"
+test_atom "Check %(subject) works for tag" "$tag_sha1" "%(subject)" "$tag_description"
+test_atom "Check %(contents) works for tag" "$tag_sha1" "%(contents)" "$tag_description"
+
+test_atom "Check %(color) gives no additional output" "$sha1" \
+ "%(objectname) %(color:green) %(objecttype)" "$sha1 $type"
+
test_expect_success \
"Reach a blob from a tag pointing to it" \
"test '$hello_content' = \"\$(git cat-file blob $tag_sha1)\""
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* Re: [PATCH v3 22/23] cat-file: tests for new atoms added
2018-02-12 8:08 ` [PATCH v3 22/23] cat-file: tests for new atoms added Olga Telezhnaya
@ 2018-02-16 14:55 ` Adam Dinwoodie
2018-02-18 22:55 ` Ramsay Jones
0 siblings, 1 reply; 115+ messages in thread
From: Adam Dinwoodie @ 2018-02-16 14:55 UTC (permalink / raw)
To: Olga Telezhnaya; +Cc: Git Mailing List
On 12 February 2018 at 08:08, Olga Telezhnaya wrote:
> Add some tests for new formatting atoms from ref-filter.
> Some of new atoms are supported automatically,
> some of them are expanded into empty string
> (because they are useless for some types of objects),
> some of them could be supported later in other patches.
This commit appears to be introducing failures in t1006 on Cygwin.
Specifically, tests 15, 36, 58 and 89, all titled "Check %(refname)
gives empty output", are failing on the pu branch at 21937aad4, and
git bisect identifies this commit, 3c1571744 ("cat-file: tests for new
atoms added", 2018-02-12), as the culprit.
I'm afraid I'm not going to have the time to investigate the failure
any further in the immediate future, but I wanted to report it
promptly in case you / someone else can see what's going wrong.
Adam
^ permalink raw reply [flat|nested] 115+ messages in thread* Re: [PATCH v3 22/23] cat-file: tests for new atoms added
2018-02-16 14:55 ` Adam Dinwoodie
@ 2018-02-18 22:55 ` Ramsay Jones
2018-02-19 1:04 ` Ramsay Jones
0 siblings, 1 reply; 115+ messages in thread
From: Ramsay Jones @ 2018-02-18 22:55 UTC (permalink / raw)
To: Adam Dinwoodie, Olga Telezhnaya
Cc: Git Mailing List, Jeff King, Christian Couder
On 16/02/18 14:55, Adam Dinwoodie wrote:
> On 12 February 2018 at 08:08, Olga Telezhnaya wrote:
>> Add some tests for new formatting atoms from ref-filter.
>> Some of new atoms are supported automatically,
>> some of them are expanded into empty string
>> (because they are useless for some types of objects),
>> some of them could be supported later in other patches.
>
> This commit appears to be introducing failures in t1006 on Cygwin.
> Specifically, tests 15, 36, 58 and 89, all titled "Check %(refname)
> gives empty output", are failing on the pu branch at 21937aad4, and
> git bisect identifies this commit, 3c1571744 ("cat-file: tests for new
> atoms added", 2018-02-12), as the culprit.
Hi Adam, Olga,
Sparse has been complaining about this 'ot/cat-batch-format' branch for
a while, so I had already noted (apart from a symbol which should be
marked as static) something that looked somewhat off - namely, the on
stack initialisation of a 'struct ref_array_item' (builtin/cat-file.c,
line 246, in function batch_object_write()).
In particular, the 'struct ref_array_item' ends with a [FLEX_ARRAY] field
for the refname, so it clearly isn't meant to be declared on the stack.
The 'ref-filter' code uses a 'constructor' function called 'new_ref_array_\
item() which, among others, takes a 'refname' parameter with which to
initialise the refname FLEX_ARRAY field.
So, on Linux, if you build with SANITIZE=address and then run that test:
$ ./t1006-cat-file.sh -i -v
Initialized empty Git repository in /home/ramsay/git/t/trash directory.t1006-cat-file/.git/
expecting success:
echo_without_newline "$hello_content" > hello &&
git update-index --add hello
ok 1 - setup
...
=================================================================
==12556==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fff132f04a8 at pc 0x7f1c1b3ca20b bp 0x7fff132f00f0 sp 0x7fff132ef898
READ of size 4 at 0x7fff132f04a8 thread T0
#0 0x7f1c1b3ca20a in __interceptor_strlen (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x7020a)
#1 0x6798cc in strbuf_addstr /home/ramsay/git/strbuf.h:273
#2 0x6798cc in quote_formatting /home/ramsay/git/ref-filter.c:496
#3 0x68325d in format_ref_array_item /home/ramsay/git/ref-filter.c:2238
#4 0x68358a in show_ref_array_item /home/ramsay/git/ref-filter.c:2262
#5 0x429866 in batch_object_write builtin/cat-file.c:252
#6 0x42b095 in batch_one_object builtin/cat-file.c:306
#7 0x42b095 in batch_objects builtin/cat-file.c:407
#8 0x42b095 in cmd_cat_file builtin/cat-file.c:528
#9 0x40d3cb in run_builtin /home/ramsay/git/git.c:346
#10 0x40d3cb in handle_builtin /home/ramsay/git/git.c:556
#11 0x40f8ae in run_argv /home/ramsay/git/git.c:608
#12 0x40f8ae in cmd_main /home/ramsay/git/git.c:685
#13 0x40b667 in main /home/ramsay/git/common-main.c:43
#14 0x7f1c1ab7982f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
#15 0x40ce38 in _start (/home/ramsay/git/git+0x40ce38)
Address 0x7fff132f04a8 is located in stack of thread T0 at offset 328 in frame
#0 0x4296ff in batch_object_write builtin/cat-file.c:245
This frame has 4 object(s):
[32, 36) 'type'
[96, 104) 'contents'
[160, 168) 'size'
[224, 328) 'item' <== Memory access at offset 328 overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
(longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow ??:0 __interceptor_strlen
Shadow bytes around the buggy address:
0x100062656040: 00 00 f3 f3 f3 f3 f3 f3 f3 f3 00 00 00 00 00 00
0x100062656050: 00 00 00 00 f1 f1 f1 f1 00 00 00 f4 f3 f3 f3 f3
0x100062656060: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
0x100062656070: 04 f4 f4 f4 f2 f2 f2 f2 00 f4 f4 f4 f2 f2 f2 f2
0x100062656080: 00 f4 f4 f4 f2 f2 f2 f2 00 00 00 00 00 00 00 00
=>0x100062656090: 00 00 00 00 00[f4]f4 f4 f3 f3 f3 f3 00 00 00 00
0x1000626560a0: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
0x1000626560b0: 04 f4 f4 f4 f2 f2 f2 f2 04 f4 f4 f4 f2 f2 f2 f2
0x1000626560c0: 04 f4 f4 f4 f2 f2 f2 f2 00 f4 f4 f4 f2 f2 f2 f2
0x1000626560d0: 00 f4 f4 f4 f2 f2 f2 f2 00 00 f4 f4 f2 f2 f2 f2
0x1000626560e0: 00 00 00 f4 f2 f2 f2 f2 00 00 00 f4 f2 f2 f2 f2
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Heap right redzone: fb
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack partial redzone: f4
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
==12556==ABORTING
Aborted (core dumped)
not ok 15 - Check %(refname) gives empty output
#
# echo "$expected" >expect &&
# echo $sha1 | git cat-file --batch-check="$atoms" >actual &&
# test_cmp expect actual
#
$
... you get a stack-overflow at that very stackframe. Incidentally, the
output from the failed test #15 on cygwin always looked the same:
$ xxd actual
00000000: a0c4 ffff 0a .....
$
So, just as an experiment, I tried changing that variable to a heap
variable and initialising it with an empty ("") refname:
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index c9d83ceff..12a743034 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -243,19 +243,20 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d
static void batch_object_write(const char *obj_name, struct batch_options *opt,
struct expand_data *data)
{
- struct ref_array_item item = {0};
+ struct ref_array_item *item;
- item.oid = data->oid;
- item.rest = data->rest;
- item.objectname = obj_name;
+ FLEX_ALLOC_STR(item, refname, "");
+ item->oid = data->oid;
+ item->rest = data->rest;
+ item->objectname = obj_name;
- if (show_ref_array_item(&item, &opt->format))
+ if (show_ref_array_item(item, &opt->format))
return;
if (!opt->buffer_output)
fflush(stdout);
if (opt->print_contents) {
- data->type = item.type;
+ data->type = item->type;
print_object_or_die(opt, data);
batch_write(opt, "\n", 1);
}
--
... and now this test passes on cygwin (and the SANITIZE build on Linux).
Of course, this is not a real fix, since this has probably only changed
the stack-overflow into an un-diagnosed heap-overflow bug! ;-)
However, the above should provide enough info for someone more familiar
with the code to implement a correct fix.
[BTW, the symbol that should be marked static is: cat_file_info, in file
ref-filter.c, line 103.]
ATB,
Ramsay Jones
^ permalink raw reply [flat|nested] 115+ messages in thread* Re: [PATCH v3 22/23] cat-file: tests for new atoms added
2018-02-18 22:55 ` Ramsay Jones
@ 2018-02-19 1:04 ` Ramsay Jones
0 siblings, 0 replies; 115+ messages in thread
From: Ramsay Jones @ 2018-02-19 1:04 UTC (permalink / raw)
To: Adam Dinwoodie, Olga Telezhnaya
Cc: Git Mailing List, Jeff King, Christian Couder
On 18/02/18 22:55, Ramsay Jones wrote:
>
>
> On 16/02/18 14:55, Adam Dinwoodie wrote:
>> On 12 February 2018 at 08:08, Olga Telezhnaya wrote:
>>> Add some tests for new formatting atoms from ref-filter.
>>> Some of new atoms are supported automatically,
>>> some of them are expanded into empty string
>>> (because they are useless for some types of objects),
>>> some of them could be supported later in other patches.
>>
>> This commit appears to be introducing failures in t1006 on Cygwin.
>> Specifically, tests 15, 36, 58 and 89, all titled "Check %(refname)
>> gives empty output", are failing on the pu branch at 21937aad4, and
>> git bisect identifies this commit, 3c1571744 ("cat-file: tests for new
>> atoms added", 2018-02-12), as the culprit.
>
> Hi Adam, Olga,
>
> Sparse has been complaining about this 'ot/cat-batch-format' branch for
> a while, so I had already noted (apart from a symbol which should be
> marked as static) something that looked somewhat off - namely, the on
> stack initialisation of a 'struct ref_array_item' (builtin/cat-file.c,
> line 246, in function batch_object_write()).
>
> In particular, the 'struct ref_array_item' ends with a [FLEX_ARRAY] field
> for the refname, so it clearly isn't meant to be declared on the stack.
> The 'ref-filter' code uses a 'constructor' function called 'new_ref_array_\
> item() which, among others, takes a 'refname' parameter with which to
> initialise the refname FLEX_ARRAY field.
>
> So, on Linux, if you build with SANITIZE=address and then run that test:
BTW, I forgot to mention that, if you run that test without -i, then
more than tests #15,36,58 and 89 will fail. In addition, tests #19-20,
#40-41, #62-63 and #93-94 failed - I did not investigate these
failures at all.
ATB,
Ramsay Jones
^ permalink raw reply [flat|nested] 115+ messages in thread
* [PATCH v3 04/23] ref-filter: make valid_atom as function parameter
2018-02-12 8:08 ` [PATCH v3 01/23] ref-filter: get rid of goto Olga Telezhnaya
` (7 preceding siblings ...)
2018-02-12 8:08 ` [PATCH v3 22/23] cat-file: tests for new atoms added Olga Telezhnaya
@ 2018-02-12 8:08 ` Olga Telezhnaya
2018-02-15 5:23 ` Jeff King
2018-02-12 8:08 ` [PATCH v3 19/23] ref-filter: make populate_value() internal again Olga Telezhnaya
` (13 subsequent siblings)
22 siblings, 1 reply; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-12 8:08 UTC (permalink / raw)
To: git
Make valid_atom as a function parameter,
there could be another variable further.
Need that for further reusing of formatting logic in cat-file.c.
We do not need to allow users to pass their own valid_atom variable in
global functions like verify_ref_format() because in the end we want to
have same set of valid atoms for all commands. But, as a first step
of migrating, I create further another version of valid_atom
for cat-file.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
ref-filter.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 9ed5e66066a7a..5e7ed0f338490 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -325,7 +325,7 @@ static void head_atom_parser(const struct ref_format *format, struct used_atom *
atom->u.head = resolve_refdup("HEAD", RESOLVE_REF_READING, NULL, NULL);
}
-static struct {
+static struct valid_atom {
const char *name;
cmp_type cmp_type;
void (*parser)(const struct ref_format *format, struct used_atom *atom, const char *arg);
@@ -396,6 +396,7 @@ struct atom_value {
* Used to parse format string and sort specifiers
*/
static int parse_ref_filter_atom(const struct ref_format *format,
+ const struct valid_atom *valid_atom, int n_atoms,
const char *atom, const char *ep)
{
const char *sp;
@@ -425,13 +426,13 @@ static int parse_ref_filter_atom(const struct ref_format *format,
atom_len = (arg ? arg : ep) - sp;
/* Is the atom a valid one? */
- for (i = 0; i < ARRAY_SIZE(valid_atom); i++) {
+ for (i = 0; i < n_atoms; i++) {
int len = strlen(valid_atom[i].name);
if (len == atom_len && !memcmp(valid_atom[i].name, sp, len))
break;
}
- if (ARRAY_SIZE(valid_atom) <= i)
+ if (n_atoms <= i)
die(_("unknown field name: %.*s"), (int)(ep-atom), atom);
/* Add it in, including the deref prefix */
@@ -708,7 +709,8 @@ int verify_ref_format(struct ref_format *format)
if (!ep)
return error(_("malformed format string %s"), sp);
/* sp points at "%(" and ep points at the closing ")" */
- at = parse_ref_filter_atom(format, sp + 2, ep);
+ at = parse_ref_filter_atom(format, valid_atom,
+ ARRAY_SIZE(valid_atom), sp + 2, ep);
cp = ep + 1;
if (skip_prefix(used_atom[at].name, "color:", &color))
@@ -2145,7 +2147,9 @@ int format_ref_array_item(struct ref_array_item *info,
if (cp < sp)
append_literal(cp, sp, &state);
if (get_ref_atom_value(info,
- parse_ref_filter_atom(format, sp + 2, ep),
+ parse_ref_filter_atom(format, valid_atom,
+ ARRAY_SIZE(valid_atom),
+ sp + 2, ep),
&atomv))
return -1;
atomv->handler(atomv, &state);
@@ -2198,7 +2202,8 @@ static int parse_sorting_atom(const char *atom)
*/
struct ref_format dummy = REF_FORMAT_INIT;
const char *end = atom + strlen(atom);
- return parse_ref_filter_atom(&dummy, atom, end);
+ return parse_ref_filter_atom(&dummy, valid_atom,
+ ARRAY_SIZE(valid_atom), atom, end);
}
/* If no sorting option is given, use refname to sort as default */
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* Re: [PATCH v3 04/23] ref-filter: make valid_atom as function parameter
2018-02-12 8:08 ` [PATCH v3 04/23] ref-filter: make valid_atom as function parameter Olga Telezhnaya
@ 2018-02-15 5:23 ` Jeff King
2018-02-15 10:03 ` Оля Тележная
0 siblings, 1 reply; 115+ messages in thread
From: Jeff King @ 2018-02-15 5:23 UTC (permalink / raw)
To: Olga Telezhnaya; +Cc: git
On Mon, Feb 12, 2018 at 08:08:54AM +0000, Olga Telezhnaya wrote:
> Make valid_atom as a function parameter,
> there could be another variable further.
> Need that for further reusing of formatting logic in cat-file.c.
>
> We do not need to allow users to pass their own valid_atom variable in
> global functions like verify_ref_format() because in the end we want to
> have same set of valid atoms for all commands. But, as a first step
> of migrating, I create further another version of valid_atom
> for cat-file.
Hmm. So I see where you're going here, but I think in the end we'd want
to have a single valid_atom list again, and we wouldn't need this.
And indeed, it looks like this goes away in patch 17. Can we
reorganize/rebase the series so that we avoid dead-end directions like
this?
-Peff
^ permalink raw reply [flat|nested] 115+ messages in thread
* Re: [PATCH v3 04/23] ref-filter: make valid_atom as function parameter
2018-02-15 5:23 ` Jeff King
@ 2018-02-15 10:03 ` Оля Тележная
0 siblings, 0 replies; 115+ messages in thread
From: Оля Тележная @ 2018-02-15 10:03 UTC (permalink / raw)
To: Jeff King; +Cc: git
2018-02-15 8:23 GMT+03:00 Jeff King <peff@peff.net>:
> On Mon, Feb 12, 2018 at 08:08:54AM +0000, Olga Telezhnaya wrote:
>
>> Make valid_atom as a function parameter,
>> there could be another variable further.
>> Need that for further reusing of formatting logic in cat-file.c.
>>
>> We do not need to allow users to pass their own valid_atom variable in
>> global functions like verify_ref_format() because in the end we want to
>> have same set of valid atoms for all commands. But, as a first step
>> of migrating, I create further another version of valid_atom
>> for cat-file.
>
> Hmm. So I see where you're going here, but I think in the end we'd want
> to have a single valid_atom list again, and we wouldn't need this.
>
> And indeed, it looks like this goes away in patch 17. Can we
> reorganize/rebase the series so that we avoid dead-end directions like
> this?
I tried to do that, but in my opinion it's easier to understand
everything in current version. I need to squash too many items so that
commits do not look atomic, and it's really hard to understand what is
going on. Now we have that helper commit that will be cancelled later,
and other logic is more clear for readers.
>
> -Peff
^ permalink raw reply [flat|nested] 115+ messages in thread
* [PATCH v3 19/23] ref-filter: make populate_value() internal again
2018-02-12 8:08 ` [PATCH v3 01/23] ref-filter: get rid of goto Olga Telezhnaya
` (8 preceding siblings ...)
2018-02-12 8:08 ` [PATCH v3 04/23] ref-filter: make valid_atom as function parameter Olga Telezhnaya
@ 2018-02-12 8:08 ` Olga Telezhnaya
2018-02-12 8:08 ` [PATCH v3 16/23] ref-filter: make cat_file_info independent Olga Telezhnaya
` (12 subsequent siblings)
22 siblings, 0 replies; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-12 8:08 UTC (permalink / raw)
To: git
Remove populate_value() from header file. We needed that
for interim step, now it could be returned back.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
ref-filter.c | 2 +-
ref-filter.h | 3 ---
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index eb53b0babdb83..8d104b567eb7c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1428,7 +1428,7 @@ static int check_and_fill_for_cat(struct ref_array_item *ref)
* Parse the object referred by ref, and grab needed value.
* Return 0 if everything was successful, -1 otherwise.
*/
-int populate_value(struct ref_array_item *ref)
+static int populate_value(struct ref_array_item *ref)
{
void *buf;
struct object *obj;
diff --git a/ref-filter.h b/ref-filter.h
index 4eaf6d0514502..115d00288fdee 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -174,9 +174,6 @@ void setup_ref_filter_porcelain_msg(void);
void pretty_print_ref(const char *name, const unsigned char *sha1,
const struct ref_format *format);
-/* Fill the values of request and prepare all data for final string creation */
-int populate_value(struct ref_array_item *ref);
-
/* Search for atom in given format. */
int is_atom_used(const struct ref_format *format, const char *atom);
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* [PATCH v3 16/23] ref-filter: make cat_file_info independent
2018-02-12 8:08 ` [PATCH v3 01/23] ref-filter: get rid of goto Olga Telezhnaya
` (9 preceding siblings ...)
2018-02-12 8:08 ` [PATCH v3 19/23] ref-filter: make populate_value() internal again Olga Telezhnaya
@ 2018-02-12 8:08 ` Olga Telezhnaya
2018-02-15 5:53 ` Jeff King
2018-02-12 8:08 ` [PATCH v3 20/23] ref-filter: unifying formatting of cat-file opts Olga Telezhnaya
` (11 subsequent siblings)
22 siblings, 1 reply; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-12 8:08 UTC (permalink / raw)
To: git
Remove connection between expand_data variable
in cat-file and in ref-filter.
It will help further to get rid of using expand_data in cat-file.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
builtin/cat-file.c | 3 ++-
ref-filter.c | 36 +++++++++++++++++++-----------------
ref-filter.h | 7 ++-----
3 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 582679f3dca2c..273b4038e3893 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -292,6 +292,7 @@ static void batch_object_write(const char *obj_name, struct batch_options *opt,
if (populate_value(&item))
return;
+ data->type = item.type;
strbuf_expand(&buf, opt->format.format, expand_format, &item);
strbuf_addch(&buf, '\n');
batch_write(opt, buf.buf, buf.len);
@@ -392,7 +393,7 @@ static int batch_objects(struct batch_options *opt)
* sha1_object_info_extended for each object.
*/
memset(&data, 0, sizeof(data));
- opt->format.cat_file_data = &data;
+ opt->format.is_cat_file = 1;
opt->format.all_objects = opt->all_objects;
verify_ref_format(&opt->format);
diff --git a/ref-filter.c b/ref-filter.c
index 104cd6aef0102..cc70bcf2bb8b1 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -100,7 +100,7 @@ static struct used_atom {
} u;
} *used_atom;
static int used_atom_cnt, need_tagged, need_symref;
-struct expand_data *cat_file_info;
+struct expand_data cat_file_info;
static void color_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *color_value)
{
@@ -255,9 +255,9 @@ static void objectname_atom_parser(const struct ref_format *format, struct used_
static void objectsize_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)
{
if (!arg)
- cat_file_info->info.sizep = &cat_file_info->size;
+ cat_file_info.info.sizep = &cat_file_info.size;
else if (!strcmp(arg, "disk"))
- cat_file_info->info.disk_sizep = &cat_file_info->disk_size;
+ cat_file_info.info.disk_sizep = &cat_file_info.disk_size;
else
die(_("urecognized %%(objectsize) argument: %s"), arg);
}
@@ -265,7 +265,7 @@ static void objectsize_atom_parser(const struct ref_format *format, struct used_
static void objecttype_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)
{
if (!arg)
- cat_file_info->info.typep = &cat_file_info->type;
+ cat_file_info.info.typep = &cat_file_info.type;
else
die(_("urecognized %%(objecttype) argument: %s"), arg);
}
@@ -273,7 +273,7 @@ static void objecttype_atom_parser(const struct ref_format *format, struct used_
static void deltabase_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)
{
if (!arg)
- cat_file_info->info.delta_base_sha1 = cat_file_info->delta_base_oid.hash;
+ cat_file_info.info.delta_base_sha1 = cat_file_info.delta_base_oid.hash;
else
die(_("urecognized %%(deltabase) argument: %s"), arg);
}
@@ -751,7 +751,7 @@ int verify_ref_format(struct ref_format *format)
{
const char *cp, *sp;
- cat_file_info = format->cat_file_data;
+ cat_file_info.is_cat_file = format->is_cat_file;
format->need_color_reset_at_eol = 0;
for (cp = format->format; *cp && (sp = find_next(cp)); ) {
const char *color, *ep = strchr(sp, ')');
@@ -761,7 +761,7 @@ int verify_ref_format(struct ref_format *format)
return error(_("malformed format string %s"), sp);
/* sp points at "%(" and ep points at the closing ")" */
- if (format->cat_file_data)
+ if (cat_file_info.is_cat_file)
at = parse_ref_filter_atom(format, valid_cat_file_atom,
ARRAY_SIZE(valid_cat_file_atom), sp + 2, ep);
else {
@@ -775,10 +775,10 @@ int verify_ref_format(struct ref_format *format)
}
if (format->need_color_reset_at_eol && !want_color(format->use_color))
format->need_color_reset_at_eol = 0;
- if (cat_file_info && format->all_objects) {
+ if (cat_file_info.is_cat_file && format->all_objects) {
struct object_info empty = OBJECT_INFO_INIT;
- if (!memcmp(&cat_file_info->info, &empty, sizeof(empty)))
- cat_file_info->skip_object_info = 1;
+ if (!memcmp(&cat_file_info.info, &empty, sizeof(empty)))
+ cat_file_info.skip_object_info = 1;
}
return 0;
}
@@ -1420,18 +1420,20 @@ static const char *get_refname(struct used_atom *atom, struct ref_array_item *re
static int check_and_fill_for_cat(struct ref_array_item *ref)
{
- if (!cat_file_info->skip_object_info &&
- sha1_object_info_extended(ref->oid.hash, &cat_file_info->info,
+ if (!cat_file_info.info.typep)
+ cat_file_info.info.typep = &cat_file_info.type;
+ if (!cat_file_info.skip_object_info &&
+ sha1_object_info_extended(ref->oid.hash, &cat_file_info.info,
OBJECT_INFO_LOOKUP_REPLACE) < 0) {
const char *e = ref->objectname;
printf("%s missing\n", e ? e : oid_to_hex(&ref->oid));
fflush(stdout);
return -1;
}
- ref->type = cat_file_info->type;
- ref->size = cat_file_info->size;
- ref->disk_size = cat_file_info->disk_size;
- ref->delta_base_oid = &cat_file_info->delta_base_oid;
+ ref->type = cat_file_info.type;
+ ref->size = cat_file_info.size;
+ ref->disk_size = cat_file_info.disk_size;
+ ref->delta_base_oid = &cat_file_info.delta_base_oid;
return 0;
}
@@ -1456,7 +1458,7 @@ int populate_value(struct ref_array_item *ref)
ref->symref = "";
}
- if (cat_file_info && check_and_fill_for_cat(ref))
+ if (cat_file_info.is_cat_file && check_and_fill_for_cat(ref))
return -1;
/* Fill in specials first */
diff --git a/ref-filter.h b/ref-filter.h
index b1c668c12428b..4eaf6d0514502 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -99,6 +99,7 @@ struct expand_data {
* optimized out.
*/
unsigned skip_object_info : 1;
+ unsigned is_cat_file : 1;
};
struct ref_format {
@@ -113,12 +114,8 @@ struct ref_format {
/* Internal state to ref-filter */
int need_color_reset_at_eol;
- /*
- * Helps to move all formatting logic from cat-file to ref-filter,
- * hopefully would be reduced later.
- */
- struct expand_data *cat_file_data;
unsigned all_objects : 1;
+ unsigned is_cat_file : 1;
};
#define REF_FORMAT_INIT { NULL, 0, -1 }
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* Re: [PATCH v3 16/23] ref-filter: make cat_file_info independent
2018-02-12 8:08 ` [PATCH v3 16/23] ref-filter: make cat_file_info independent Olga Telezhnaya
@ 2018-02-15 5:53 ` Jeff King
2018-02-15 10:33 ` Оля Тележная
0 siblings, 1 reply; 115+ messages in thread
From: Jeff King @ 2018-02-15 5:53 UTC (permalink / raw)
To: Olga Telezhnaya; +Cc: git
On Mon, Feb 12, 2018 at 08:08:54AM +0000, Olga Telezhnaya wrote:
> Remove connection between expand_data variable
> in cat-file and in ref-filter.
> It will help further to get rid of using expand_data in cat-file.
I have to admit I'm confused at this point about what is_cat_file is
for, or even why we need cat_file_data. Shouldn't these items be handled
by their matching ref-filter atoms at this point?
-Peff
^ permalink raw reply [flat|nested] 115+ messages in thread
* Re: [PATCH v3 16/23] ref-filter: make cat_file_info independent
2018-02-15 5:53 ` Jeff King
@ 2018-02-15 10:33 ` Оля Тележная
2018-02-16 21:54 ` Jeff King
0 siblings, 1 reply; 115+ messages in thread
From: Оля Тележная @ 2018-02-15 10:33 UTC (permalink / raw)
To: Jeff King; +Cc: git
2018-02-15 8:53 GMT+03:00 Jeff King <peff@peff.net>:
> On Mon, Feb 12, 2018 at 08:08:54AM +0000, Olga Telezhnaya wrote:
>
>> Remove connection between expand_data variable
>> in cat-file and in ref-filter.
>> It will help further to get rid of using expand_data in cat-file.
>
> I have to admit I'm confused at this point about what is_cat_file is
> for, or even why we need cat_file_data. Shouldn't these items be handled
> by their matching ref-filter atoms at this point?
We discussed that earlier outside of mailing list, and I even tried to
implement that idea and spent a couple of days to prove that it's not
possible.
The problem is that the list of atoms is made dynamically, and we
can't store pointers to any values in each atom. That's why we need
separate cat_file_info variable that is outside of main atom list.
We also need is_cat_file because we still have some part of logic that
is different for cat-file and for all other commands, and sometimes we
need to know that information.
>
> -Peff
^ permalink raw reply [flat|nested] 115+ messages in thread
* Re: [PATCH v3 16/23] ref-filter: make cat_file_info independent
2018-02-15 10:33 ` Оля Тележная
@ 2018-02-16 21:54 ` Jeff King
0 siblings, 0 replies; 115+ messages in thread
From: Jeff King @ 2018-02-16 21:54 UTC (permalink / raw)
To: Оля
Тележная
Cc: git
On Thu, Feb 15, 2018 at 01:33:25PM +0300, Оля Тележная wrote:
> 2018-02-15 8:53 GMT+03:00 Jeff King <peff@peff.net>:
> > On Mon, Feb 12, 2018 at 08:08:54AM +0000, Olga Telezhnaya wrote:
> >
> >> Remove connection between expand_data variable
> >> in cat-file and in ref-filter.
> >> It will help further to get rid of using expand_data in cat-file.
> >
> > I have to admit I'm confused at this point about what is_cat_file is
> > for, or even why we need cat_file_data. Shouldn't these items be handled
> > by their matching ref-filter atoms at this point?
>
> We discussed that earlier outside of mailing list, and I even tried to
> implement that idea and spent a couple of days to prove that it's not
> possible.
> The problem is that the list of atoms is made dynamically, and we
> can't store pointers to any values in each atom. That's why we need
> separate cat_file_info variable that is outside of main atom list.
> We also need is_cat_file because we still have some part of logic that
> is different for cat-file and for all other commands, and sometimes we
> need to know that information.
OK, right, I forgot about the pointers thing. This is definitely the
sort of design decision that should go into the commit message.
I went over our off-list discussion again to refresh my memory. Let me
try to summarize a bit (for myself, or for other readerss). Naively, one
would hope that you could do something like:
1. While parsing the %(objectsize) atom, add a pointer like:
the_object_info->sizep = &atom.size;
2. Later when fulfilling the request for a specific object, if
the_object_info is not empty, then call sha1_object_info() with it
to hold the results.
3. Read the value out of atom.size when assembling the formatted
string.
But that has two problems:
a. During parsing, we're resizing the atom array, so the &atom.size
pointer is subject to change. This is the issue you mentioned
above. I think we could solve it by taking a pass over the atoms
after parsing and filling in the pointers then. But...
b. Another problem is that the same atom may appear multiple times. So
parsing "%(objectsize) %(objectsize)", we can't point
the_object_info at _both_ of them. We need our call to
sha1_object_info() to store the result in a single location, and
then as we format each atom they can both use that (which works
even if they might format it differently, say if you had
"%(deltabase) %(deltabase:abbrev)".
So we do need some kind of global storage to hold these results. This is
sort of the same problem we have with parsing the objects at all. There
we get by without a global because all of the logic is crammed into
populate_value(). In theory we could do the same thing here, but there
are just a lot of different items we might be asking for. So instead of
"need_tagged", you'd have a multitude of need_objectsize_disk,
need_deltabase, etc, which gets unwieldy. We may as well fill in a
"struct object_info".
So OK, I agree that something like "struct expand_data" is needed.
But what seems non-ideal to me is the way that ref-filter depends on it
in this cat-file-specific way. If it needs those values it should be
calling sha1_object_info(). And if it doesn't, then it can skip the
call. It shouldn't need to know about cat-file at all.
And likewise cat-file should not know about expand_data. And I think
that's true by the end of the series, though the steps in the middle
left me a little confused.
-Peff
^ permalink raw reply [flat|nested] 115+ messages in thread
* [PATCH v3 20/23] ref-filter: unifying formatting of cat-file opts
2018-02-12 8:08 ` [PATCH v3 01/23] ref-filter: get rid of goto Olga Telezhnaya
` (10 preceding siblings ...)
2018-02-12 8:08 ` [PATCH v3 16/23] ref-filter: make cat_file_info independent Olga Telezhnaya
@ 2018-02-12 8:08 ` Olga Telezhnaya
2018-02-15 5:56 ` Jeff King
2018-02-12 8:08 ` [PATCH v3 09/23] cat-file: start use ref_array_item struct Olga Telezhnaya
` (10 subsequent siblings)
22 siblings, 1 reply; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-12 8:08 UTC (permalink / raw)
To: git
cat-file options are now filled by general logic.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
ref-filter.c | 33 ++++++++++++++++-----------------
1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 8d104b567eb7c..5781416cf9126 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -824,8 +824,12 @@ static void grab_common_values(struct atom_value *val, int deref, struct object
else if (!strcmp(name, "objectsize")) {
v->value = sz;
v->s = xstrfmt("%lu", sz);
- }
- else if (deref)
+ } else if (!strcmp(name, "objectsize:disk")) {
+ if (cat_file_info.is_cat_file) {
+ v->value = cat_file_info.disk_size;
+ v->s = xstrfmt("%"PRIuMAX, (uintmax_t)v->value);
+ }
+ } else if (deref)
grab_objectname(name, obj->oid.hash, v, &used_atom[i]);
}
}
@@ -1465,21 +1469,7 @@ static int populate_value(struct ref_array_item *ref)
name++;
}
- if (cat_file_info.is_cat_file) {
- if (starts_with(name, "objectname"))
- v->s = oid_to_hex(&ref->oid);
- else if (starts_with(name, "objecttype"))
- v->s = typename(ref->type);
- else if (starts_with(name, "objectsize")) {
- v->s = xstrfmt("%lu", ref->size);
- } else if (starts_with(name, "objectsize:disk")) {
- v->s = xstrfmt("%"PRIuMAX, (uintmax_t)ref->disk_size);
- } else if (starts_with(name, "rest"))
- v->s = ref->rest;
- else if (starts_with(name, "deltabase"))
- v->s = xstrdup(oid_to_hex(ref->delta_base_oid));
- continue;
- } else if (starts_with(name, "refname"))
+ if (starts_with(name, "refname"))
refname = get_refname(atom, ref);
else if (starts_with(name, "symref"))
refname = get_symref(atom, ref);
@@ -1535,6 +1525,15 @@ static int populate_value(struct ref_array_item *ref)
else
v->s = " ";
continue;
+ } else if (starts_with(name, "rest")) {
+ v->s = ref->rest ? ref->rest : "";
+ continue;
+ } else if (starts_with(name, "deltabase")) {
+ if (ref->delta_base_oid)
+ v->s = xstrdup(oid_to_hex(ref->delta_base_oid));
+ else
+ v->s = "";
+ continue;
} else if (starts_with(name, "align")) {
v->handler = align_atom_handler;
continue;
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* Re: [PATCH v3 20/23] ref-filter: unifying formatting of cat-file opts
2018-02-12 8:08 ` [PATCH v3 20/23] ref-filter: unifying formatting of cat-file opts Olga Telezhnaya
@ 2018-02-15 5:56 ` Jeff King
2018-02-15 10:34 ` Оля Тележная
0 siblings, 1 reply; 115+ messages in thread
From: Jeff King @ 2018-02-15 5:56 UTC (permalink / raw)
To: Olga Telezhnaya; +Cc: git
On Mon, Feb 12, 2018 at 08:08:54AM +0000, Olga Telezhnaya wrote:
> cat-file options are now filled by general logic.
Yay.
One puzzling thing:
> diff --git a/ref-filter.c b/ref-filter.c
> index 8d104b567eb7c..5781416cf9126 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -824,8 +824,12 @@ static void grab_common_values(struct atom_value *val, int deref, struct object
> else if (!strcmp(name, "objectsize")) {
> v->value = sz;
> v->s = xstrfmt("%lu", sz);
> - }
> - else if (deref)
> + } else if (!strcmp(name, "objectsize:disk")) {
> + if (cat_file_info.is_cat_file) {
> + v->value = cat_file_info.disk_size;
> + v->s = xstrfmt("%"PRIuMAX, (uintmax_t)v->value);
> + }
> + } else if (deref)
Why do we care about is_cat_file here. Shouldn't:
git for-each-ref --format='%(objectsize:disk)'
work? I.e., shouldn't the cat_file_info.disk_size variable be held
somewhere in a used_atom struct?
-Peff
^ permalink raw reply [flat|nested] 115+ messages in thread* Re: [PATCH v3 20/23] ref-filter: unifying formatting of cat-file opts
2018-02-15 5:56 ` Jeff King
@ 2018-02-15 10:34 ` Оля Тележная
2018-02-15 18:31 ` Junio C Hamano
0 siblings, 1 reply; 115+ messages in thread
From: Оля Тележная @ 2018-02-15 10:34 UTC (permalink / raw)
To: Jeff King; +Cc: git
2018-02-15 8:56 GMT+03:00 Jeff King <peff@peff.net>:
> On Mon, Feb 12, 2018 at 08:08:54AM +0000, Olga Telezhnaya wrote:
>
>> cat-file options are now filled by general logic.
>
> Yay.
>
> One puzzling thing:
>
>> diff --git a/ref-filter.c b/ref-filter.c
>> index 8d104b567eb7c..5781416cf9126 100644
>> --- a/ref-filter.c
>> +++ b/ref-filter.c
>> @@ -824,8 +824,12 @@ static void grab_common_values(struct atom_value *val, int deref, struct object
>> else if (!strcmp(name, "objectsize")) {
>> v->value = sz;
>> v->s = xstrfmt("%lu", sz);
>> - }
>> - else if (deref)
>> + } else if (!strcmp(name, "objectsize:disk")) {
>> + if (cat_file_info.is_cat_file) {
>> + v->value = cat_file_info.disk_size;
>> + v->s = xstrfmt("%"PRIuMAX, (uintmax_t)v->value);
>> + }
>> + } else if (deref)
>
> Why do we care about is_cat_file here. Shouldn't:
>
> git for-each-ref --format='%(objectsize:disk)'
>
> work? I.e., shouldn't the cat_file_info.disk_size variable be held
> somewhere in a used_atom struct?
At that point - no.
I think it sounds like other separate task to add this functionality
and to test it properly.
>
> -Peff
^ permalink raw reply [flat|nested] 115+ messages in thread* Re: [PATCH v3 20/23] ref-filter: unifying formatting of cat-file opts
2018-02-15 10:34 ` Оля Тележная
@ 2018-02-15 18:31 ` Junio C Hamano
0 siblings, 0 replies; 115+ messages in thread
From: Junio C Hamano @ 2018-02-15 18:31 UTC (permalink / raw)
To: Оля
Тележная
Cc: Jeff King, git
Оля Тележная <olyatelezhnaya@gmail.com> writes:
>>> - else if (deref)
>>> + } else if (!strcmp(name, "objectsize:disk")) {
>>> + if (cat_file_info.is_cat_file) {
>>> + v->value = cat_file_info.disk_size;
>>> + v->s = xstrfmt("%"PRIuMAX, (uintmax_t)v->value);
>>> + }
>>> + } else if (deref)
>>
>> Why do we care about is_cat_file here. Shouldn't:
>>
>> git for-each-ref --format='%(objectsize:disk)'
>>
>> work? I.e., shouldn't the cat_file_info.disk_size variable be held
>> somewhere in a used_atom struct?
>
> At that point - no.
> I think it sounds like other separate task to add this functionality
> and to test it properly.
What does "that point" refer to? This point at 20th patch in the
23-patch series it is not premature, but it will become feasible in
later steps?
As Peff already said in his review on earlier steps like 4/23 and
7/23, I too found the series quite confusing and felt as if I was
watching somebody stumbling in all directions in the dark in the
earlier steps in the series before deciding to go in one direction.
^ permalink raw reply [flat|nested] 115+ messages in thread
* [PATCH v3 09/23] cat-file: start use ref_array_item struct
2018-02-12 8:08 ` [PATCH v3 01/23] ref-filter: get rid of goto Olga Telezhnaya
` (11 preceding siblings ...)
2018-02-12 8:08 ` [PATCH v3 20/23] ref-filter: unifying formatting of cat-file opts Olga Telezhnaya
@ 2018-02-12 8:08 ` Olga Telezhnaya
2018-02-15 5:40 ` Jeff King
2018-02-12 8:08 ` [PATCH v3 14/23] ref_filter: add is_atom_used function Olga Telezhnaya
` (9 subsequent siblings)
22 siblings, 1 reply; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-12 8:08 UTC (permalink / raw)
To: git
Moving from using expand_data to ref_array_item structure.
That helps us to reuse functions from ref-filter easier.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
builtin/cat-file.c | 32 ++++++++++++++++++++------------
ref-filter.h | 5 +++++
2 files changed, 25 insertions(+), 12 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 67e7790d2f319..5b7bc34f1ec6d 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -183,27 +183,27 @@ static int is_atom(const char *atom, const char *s, int slen)
}
static void expand_atom(struct strbuf *sb, const char *atom, int len,
- struct expand_data *data)
+ struct ref_array_item *item)
{
if (is_atom("objectname", atom, len))
- strbuf_addstr(sb, oid_to_hex(&data->oid));
+ strbuf_addstr(sb, oid_to_hex(&item->objectname));
else if (is_atom("objecttype", atom, len))
- strbuf_addstr(sb, typename(data->type));
+ strbuf_addstr(sb, typename(item->type));
else if (is_atom("objectsize", atom, len))
- strbuf_addf(sb, "%lu", data->size);
+ strbuf_addf(sb, "%lu", item->size);
else if (is_atom("objectsize:disk", atom, len))
- strbuf_addf(sb, "%"PRIuMAX, (uintmax_t)data->disk_size);
+ strbuf_addf(sb, "%"PRIuMAX, (uintmax_t)item->disk_size);
else if (is_atom("rest", atom, len)) {
- if (data->rest)
- strbuf_addstr(sb, data->rest);
+ if (item->rest)
+ strbuf_addstr(sb, item->rest);
} else if (is_atom("deltabase", atom, len))
- strbuf_addstr(sb, oid_to_hex(&data->delta_base_oid));
+ strbuf_addstr(sb, oid_to_hex(item->delta_base_oid));
}
-static size_t expand_format(struct strbuf *sb, const char *start, void *vdata)
+static size_t expand_format(struct strbuf *sb, const char *start, void *data)
{
const char *end;
- struct expand_data *data = vdata;
+ struct ref_array_item *item = data;
if (*start != '(')
return 0;
@@ -211,7 +211,7 @@ static size_t expand_format(struct strbuf *sb, const char *start, void *vdata)
if (!end)
die("format element '%s' does not end in ')'", start);
- expand_atom(sb, start + 1, end - start - 1, data);
+ expand_atom(sb, start + 1, end - start - 1, item);
return end - start + 1;
}
@@ -283,6 +283,7 @@ static void batch_object_write(const char *obj_name, struct batch_options *opt,
struct expand_data *data)
{
struct strbuf buf = STRBUF_INIT;
+ struct ref_array_item item = {0};
if (!data->skip_object_info &&
sha1_object_info_extended(data->oid.hash, &data->info,
@@ -293,7 +294,14 @@ static void batch_object_write(const char *obj_name, struct batch_options *opt,
return;
}
- strbuf_expand(&buf, opt->format.format, expand_format, data);
+ item.objectname = data->oid;
+ item.type = data->type;
+ item.size = data->size;
+ item.disk_size = data->disk_size;
+ item.rest = data->rest;
+ item.delta_base_oid = &data->delta_base_oid;
+
+ strbuf_expand(&buf, opt->format.format, expand_format, &item);
strbuf_addch(&buf, '\n');
batch_write(opt, buf.buf, buf.len);
strbuf_release(&buf);
diff --git a/ref-filter.h b/ref-filter.h
index 52e07dbe6864a..781921d4e0978 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -40,6 +40,11 @@ struct ref_array_item {
const char *symref;
struct commit *commit;
struct atom_value *value;
+ enum object_type type;
+ unsigned long size;
+ off_t disk_size;
+ const char *rest;
+ struct object_id *delta_base_oid;
char refname[FLEX_ARRAY];
};
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* Re: [PATCH v3 09/23] cat-file: start use ref_array_item struct
2018-02-12 8:08 ` [PATCH v3 09/23] cat-file: start use ref_array_item struct Olga Telezhnaya
@ 2018-02-15 5:40 ` Jeff King
2018-02-15 10:13 ` Оля Тележная
0 siblings, 1 reply; 115+ messages in thread
From: Jeff King @ 2018-02-15 5:40 UTC (permalink / raw)
To: Olga Telezhnaya; +Cc: git
On Mon, Feb 12, 2018 at 08:08:54AM +0000, Olga Telezhnaya wrote:
> Moving from using expand_data to ref_array_item structure.
> That helps us to reuse functions from ref-filter easier.
This one feels weird. The point of a ref_array_item is for the caller to
feed data into the ref-filter formatting code (usually that data comes
from an earlier call to filter_refs(), but in the new cat-file we'd
presumably feed single items).
But here we're adding a bunch of fields for items that we'd expect the
format code to compute itself.
-Peff
^ permalink raw reply [flat|nested] 115+ messages in thread
* Re: [PATCH v3 09/23] cat-file: start use ref_array_item struct
2018-02-15 5:40 ` Jeff King
@ 2018-02-15 10:13 ` Оля Тележная
0 siblings, 0 replies; 115+ messages in thread
From: Оля Тележная @ 2018-02-15 10:13 UTC (permalink / raw)
To: Jeff King; +Cc: git
2018-02-15 8:40 GMT+03:00 Jeff King <peff@peff.net>:
> On Mon, Feb 12, 2018 at 08:08:54AM +0000, Olga Telezhnaya wrote:
>
>> Moving from using expand_data to ref_array_item structure.
>> That helps us to reuse functions from ref-filter easier.
>
> This one feels weird. The point of a ref_array_item is for the caller to
> feed data into the ref-filter formatting code (usually that data comes
> from an earlier call to filter_refs(), but in the new cat-file we'd
> presumably feed single items).
>
> But here we're adding a bunch of fields for items that we'd expect the
> format code to compute itself.
It would be changed later, it's just the addition of new structure
that we have never used in cat-file before.
>
> -Peff
^ permalink raw reply [flat|nested] 115+ messages in thread
* [PATCH v3 14/23] ref_filter: add is_atom_used function
2018-02-12 8:08 ` [PATCH v3 01/23] ref-filter: get rid of goto Olga Telezhnaya
` (12 preceding siblings ...)
2018-02-12 8:08 ` [PATCH v3 09/23] cat-file: start use ref_array_item struct Olga Telezhnaya
@ 2018-02-12 8:08 ` Olga Telezhnaya
2018-02-15 5:49 ` Jeff King
2018-02-12 8:08 ` [PATCH v3 08/23] ref-filter: reuse parse_ref_filter_atom() Olga Telezhnaya
` (8 subsequent siblings)
22 siblings, 1 reply; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-12 8:08 UTC (permalink / raw)
To: git
Delete all items related to split_on_whitespace from ref-filter
and add new function for handling the logic.
Now cat-file could invoke that function to implementing its logic.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
builtin/cat-file.c | 8 +++-----
ref-filter.c | 17 +++++++++++++++--
ref-filter.h | 10 +++-------
3 files changed, 21 insertions(+), 14 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 6db57e3533806..3a49b55a1cc2e 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -382,8 +382,7 @@ static int batch_objects(struct batch_options *opt)
{
struct strbuf buf = STRBUF_INIT;
struct expand_data data;
- int save_warning;
- int retval = 0;
+ int save_warning, is_rest, retval = 0;
if (!opt->format.format)
opt->format.format = "%(objectname) %(objecttype) %(objectsize)";
@@ -395,8 +394,6 @@ static int batch_objects(struct batch_options *opt)
memset(&data, 0, sizeof(data));
opt->format.cat_file_data = &data;
verify_ref_format(&opt->format);
- if (opt->cmdmode)
- data.split_on_whitespace = 1;
if (opt->all_objects) {
struct object_info empty = OBJECT_INFO_INIT;
@@ -435,9 +432,10 @@ static int batch_objects(struct batch_options *opt)
*/
save_warning = warn_on_object_refname_ambiguity;
warn_on_object_refname_ambiguity = 0;
+ is_rest = opt->cmdmode || is_atom_used(&opt->format, "rest");
while (strbuf_getline(&buf, stdin) != EOF) {
- if (data.split_on_whitespace) {
+ if (is_rest) {
/*
* Split at first whitespace, tying off the beginning
* of the string and saving the remainder (or NULL) in
diff --git a/ref-filter.c b/ref-filter.c
index 34a54db168265..4adeea6aad0da 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -493,8 +493,6 @@ static int parse_ref_filter_atom(const struct ref_format *format,
need_tagged = 1;
if (!strcmp(valid_atom[i].name, "symref"))
need_symref = 1;
- if (cat_file_info && !strcmp(valid_atom[i].name, "rest"))
- cat_file_info->split_on_whitespace = 1;
return at;
}
@@ -730,6 +728,21 @@ static const char *find_next(const char *cp)
return NULL;
}
+/* Search for atom in given format. */
+int is_atom_used(const struct ref_format *format, const char *atom)
+{
+ const char *cp, *sp;
+ for (cp = format->format; *cp && (sp = find_next(cp)); ) {
+ const char *ep = strchr(sp, ')');
+ int atom_len = ep - sp - 2;
+ sp += 2;
+ if (atom_len == strlen(atom) && !memcmp(sp, atom, atom_len))
+ return 1;
+ cp = ep + 1;
+ }
+ return 0;
+}
+
/*
* Make sure the format string is well formed, and parse out
* the used atoms.
diff --git a/ref-filter.h b/ref-filter.h
index 5c6e019998716..fffc443726e28 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -86,13 +86,6 @@ struct expand_data {
const char *rest;
struct object_id delta_base_oid;
- /*
- * Whether to split the input on whitespace before feeding it to
- * get_sha1; this is decided during the mark_query phase based on
- * whether we have a %(rest) token in our format.
- */
- int split_on_whitespace;
-
/*
* After a mark_query run, this object_info is set up to be
* passed to sha1_object_info_extended. It will point to the data
@@ -186,4 +179,7 @@ void pretty_print_ref(const char *name, const unsigned char *sha1,
/* Fill the values of request and prepare all data for final string creation */
int populate_value(struct ref_array_item *ref);
+/* Search for atom in given format. */
+int is_atom_used(const struct ref_format *format, const char *atom);
+
#endif /* REF_FILTER_H */
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* Re: [PATCH v3 14/23] ref_filter: add is_atom_used function
2018-02-12 8:08 ` [PATCH v3 14/23] ref_filter: add is_atom_used function Olga Telezhnaya
@ 2018-02-15 5:49 ` Jeff King
2018-02-15 10:20 ` Оля Тележная
0 siblings, 1 reply; 115+ messages in thread
From: Jeff King @ 2018-02-15 5:49 UTC (permalink / raw)
To: Olga Telezhnaya; +Cc: git
On Mon, Feb 12, 2018 at 08:08:54AM +0000, Olga Telezhnaya wrote:
> Delete all items related to split_on_whitespace from ref-filter
> and add new function for handling the logic.
> Now cat-file could invoke that function to implementing its logic.
OK, this is a good direction. I think in a more compact series we'd
avoid moving the split-on-whitespace bits over to ref-filter in the
first place, and have two commits:
- one early in the series adding is_atom_used()
- one late in the series switching cat-file over to is_atom_used() as
part of the conversion to ref-filter
> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index 6db57e3533806..3a49b55a1cc2e 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -382,8 +382,7 @@ static int batch_objects(struct batch_options *opt)
> {
> struct strbuf buf = STRBUF_INIT;
> struct expand_data data;
> - int save_warning;
> - int retval = 0;
> + int save_warning, is_rest, retval = 0;
Try to avoid reformatting existing code that you're not otherwise
touching, as it makes the diff noisier. Just adding "int is_rest" would
make this easier to review.
I also think the variable name should probably still be
"split_on_whitespace". It's set based on whether we saw a "%(rest)"
atom, but ultimately we'll use it to decide whether to split.
-Peff
^ permalink raw reply [flat|nested] 115+ messages in thread* Re: [PATCH v3 14/23] ref_filter: add is_atom_used function
2018-02-15 5:49 ` Jeff King
@ 2018-02-15 10:20 ` Оля Тележная
0 siblings, 0 replies; 115+ messages in thread
From: Оля Тележная @ 2018-02-15 10:20 UTC (permalink / raw)
To: Jeff King; +Cc: git
2018-02-15 8:49 GMT+03:00 Jeff King <peff@peff.net>:
> On Mon, Feb 12, 2018 at 08:08:54AM +0000, Olga Telezhnaya wrote:
>
>> Delete all items related to split_on_whitespace from ref-filter
>> and add new function for handling the logic.
>> Now cat-file could invoke that function to implementing its logic.
>
> OK, this is a good direction. I think in a more compact series we'd
> avoid moving the split-on-whitespace bits over to ref-filter in the
> first place, and have two commits:
>
> - one early in the series adding is_atom_used()
>
> - one late in the series switching cat-file over to is_atom_used() as
> part of the conversion to ref-filter
>
>> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
>> index 6db57e3533806..3a49b55a1cc2e 100644
>> --- a/builtin/cat-file.c
>> +++ b/builtin/cat-file.c
>> @@ -382,8 +382,7 @@ static int batch_objects(struct batch_options *opt)
>> {
>> struct strbuf buf = STRBUF_INIT;
>> struct expand_data data;
>> - int save_warning;
>> - int retval = 0;
>> + int save_warning, is_rest, retval = 0;
>
> Try to avoid reformatting existing code that you're not otherwise
> touching, as it makes the diff noisier. Just adding "int is_rest" would
> make this easier to review.
>
> I also think the variable name should probably still be
> "split_on_whitespace". It's set based on whether we saw a "%(rest)"
> atom, but ultimately we'll use it to decide whether to split.
OK, I will fix that.
>
> -Peff
^ permalink raw reply [flat|nested] 115+ messages in thread
* [PATCH v3 08/23] ref-filter: reuse parse_ref_filter_atom()
2018-02-12 8:08 ` [PATCH v3 01/23] ref-filter: get rid of goto Olga Telezhnaya
` (13 preceding siblings ...)
2018-02-12 8:08 ` [PATCH v3 14/23] ref_filter: add is_atom_used function Olga Telezhnaya
@ 2018-02-12 8:08 ` Olga Telezhnaya
2018-02-15 5:37 ` Jeff King
2018-02-12 8:08 ` [PATCH v3 11/23] ref-filter: rename field in ref_array_item stuct Olga Telezhnaya
` (7 subsequent siblings)
22 siblings, 1 reply; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-12 8:08 UTC (permalink / raw)
To: git
Continue migrating formatting logic from cat-file to ref-filter.
Reuse parse_ref_filter_atom() for unifying all processes in ref-filter
and further removing of mark_atom_in_object_info().
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
ref-filter.c | 34 +++++++++++++++++++++++++---------
1 file changed, 25 insertions(+), 9 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index ff87e632f463c..5c75259b1ab8c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -100,6 +100,7 @@ static struct used_atom {
} u;
} *used_atom;
static int used_atom_cnt, need_tagged, need_symref;
+struct expand_data *cat_file_info;
static void color_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *color_value)
{
@@ -251,6 +252,16 @@ static void objectname_atom_parser(const struct ref_format *format, struct used_
die(_("unrecognized %%(objectname) argument: %s"), arg);
}
+static void objectsize_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)
+{
+ if (!arg)
+ ; /* default to normal object size */
+ else if (!strcmp(arg, "disk"))
+ cat_file_info->info.disk_sizep = &cat_file_info->disk_size;
+ else
+ die(_("urecognized %%(objectsize) argument: %s"), arg);
+}
+
static void refname_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)
{
refname_atom_parser_internal(&atom->u.refname, arg, atom->name);
@@ -371,6 +382,14 @@ static struct valid_atom {
{ "else" },
};
+static struct valid_atom valid_cat_file_atom[] = {
+ { "objectname" },
+ { "objecttype" },
+ { "objectsize", FIELD_ULONG, objectsize_atom_parser },
+ { "rest" },
+ { "deltabase" },
+};
+
#define REF_FORMATTING_STATE_INIT { 0, NULL }
struct ref_formatting_stack {
@@ -401,20 +420,14 @@ static int is_atom(const char *atom, const char *s, int slen)
static void mark_atom_in_object_info(const char *atom, int len,
struct expand_data *data)
{
- if (is_atom("objectname", atom, len))
- ; /* do nothing */
- else if (is_atom("objecttype", atom, len))
+ if (is_atom("objecttype", atom, len))
data->info.typep = &data->type;
else if (is_atom("objectsize", atom, len))
data->info.sizep = &data->size;
- else if (is_atom("objectsize:disk", atom, len))
- data->info.disk_sizep = &data->disk_size;
else if (is_atom("rest", atom, len))
data->split_on_whitespace = 1;
else if (is_atom("deltabase", atom, len))
data->info.delta_base_sha1 = data->delta_base_oid.hash;
- else
- die("unknown format element: %.*s", len, atom);
}
/*
@@ -483,6 +496,8 @@ static int parse_ref_filter_atom(const struct ref_format *format,
need_tagged = 1;
if (!strcmp(valid_atom[i].name, "symref"))
need_symref = 1;
+ if (cat_file_info)
+ mark_atom_in_object_info(atom, atom_len, cat_file_info);
return at;
}
@@ -726,6 +741,7 @@ int verify_ref_format(struct ref_format *format)
{
const char *cp, *sp;
+ cat_file_info = format->cat_file_data;
format->need_color_reset_at_eol = 0;
for (cp = format->format; *cp && (sp = find_next(cp)); ) {
const char *color, *ep = strchr(sp, ')');
@@ -736,8 +752,8 @@ int verify_ref_format(struct ref_format *format)
/* sp points at "%(" and ep points at the closing ")" */
if (format->cat_file_data)
- mark_atom_in_object_info(sp + 2, ep - sp - 2,
- format->cat_file_data);
+ at = parse_ref_filter_atom(format, valid_cat_file_atom,
+ ARRAY_SIZE(valid_cat_file_atom), sp + 2, ep);
else {
at = parse_ref_filter_atom(format, valid_atom,
ARRAY_SIZE(valid_atom), sp + 2, ep);
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* Re: [PATCH v3 08/23] ref-filter: reuse parse_ref_filter_atom()
2018-02-12 8:08 ` [PATCH v3 08/23] ref-filter: reuse parse_ref_filter_atom() Olga Telezhnaya
@ 2018-02-15 5:37 ` Jeff King
2018-02-15 10:11 ` Оля Тележная
0 siblings, 1 reply; 115+ messages in thread
From: Jeff King @ 2018-02-15 5:37 UTC (permalink / raw)
To: Olga Telezhnaya; +Cc: git
On Mon, Feb 12, 2018 at 08:08:54AM +0000, Olga Telezhnaya wrote:
> Continue migrating formatting logic from cat-file to ref-filter.
> Reuse parse_ref_filter_atom() for unifying all processes in ref-filter
> and further removing of mark_atom_in_object_info().
OK, now it looks we're moving in a good direction.
One thing that puzzles me:
> @@ -401,20 +420,14 @@ static int is_atom(const char *atom, const char *s, int slen)
> static void mark_atom_in_object_info(const char *atom, int len,
> struct expand_data *data)
> {
> - if (is_atom("objectname", atom, len))
> - ; /* do nothing */
> - else if (is_atom("objecttype", atom, len))
> + if (is_atom("objecttype", atom, len))
> data->info.typep = &data->type;
> else if (is_atom("objectsize", atom, len))
> data->info.sizep = &data->size;
> - else if (is_atom("objectsize:disk", atom, len))
> - data->info.disk_sizep = &data->disk_size;
> else if (is_atom("rest", atom, len))
> data->split_on_whitespace = 1;
> else if (is_atom("deltabase", atom, len))
> data->info.delta_base_sha1 = data->delta_base_oid.hash;
> - else
> - die("unknown format element: %.*s", len, atom);
> }
Why do some of these atoms go away and not others? It seems like we're
now relying on ref-filter to parse some of the common ones using its
existing atom-parser. But wouldn't it have objecttype and objectsize
already, then?
-Peff
^ permalink raw reply [flat|nested] 115+ messages in thread* Re: [PATCH v3 08/23] ref-filter: reuse parse_ref_filter_atom()
2018-02-15 5:37 ` Jeff King
@ 2018-02-15 10:11 ` Оля Тележная
0 siblings, 0 replies; 115+ messages in thread
From: Оля Тележная @ 2018-02-15 10:11 UTC (permalink / raw)
To: Jeff King; +Cc: git
2018-02-15 8:37 GMT+03:00 Jeff King <peff@peff.net>:
> On Mon, Feb 12, 2018 at 08:08:54AM +0000, Olga Telezhnaya wrote:
>
>> Continue migrating formatting logic from cat-file to ref-filter.
>> Reuse parse_ref_filter_atom() for unifying all processes in ref-filter
>> and further removing of mark_atom_in_object_info().
>
> OK, now it looks we're moving in a good direction.
>
> One thing that puzzles me:
>
>> @@ -401,20 +420,14 @@ static int is_atom(const char *atom, const char *s, int slen)
>> static void mark_atom_in_object_info(const char *atom, int len,
>> struct expand_data *data)
>> {
>> - if (is_atom("objectname", atom, len))
>> - ; /* do nothing */
>> - else if (is_atom("objecttype", atom, len))
>> + if (is_atom("objecttype", atom, len))
>> data->info.typep = &data->type;
>> else if (is_atom("objectsize", atom, len))
>> data->info.sizep = &data->size;
>> - else if (is_atom("objectsize:disk", atom, len))
>> - data->info.disk_sizep = &data->disk_size;
>> else if (is_atom("rest", atom, len))
>> data->split_on_whitespace = 1;
>> else if (is_atom("deltabase", atom, len))
>> data->info.delta_base_sha1 = data->delta_base_oid.hash;
>> - else
>> - die("unknown format element: %.*s", len, atom);
>> }
>
> Why do some of these atoms go away and not others?
I deleted "objectname" because we were doing nothing there;
"objectsize:disk" because we have its own parser function;
"die" because ref-filter has its own checker whether the atom is valid or not.
I left all others because I haven't supported them at that point. This
whole function will be removed later.
> It seems like we're
> now relying on ref-filter to parse some of the common ones using its
> existing atom-parser. But wouldn't it have objecttype and objectsize
> already, then?
We haven't migrated enough to ref-filter at this point and we can't
reuse general ref-filter logic about filling the fields. So, we still
need to have our own function for doing that. Anyway, as I said
earlier, we will reach that status in the end of the patch: this
function would be deleted and we will use general ref-filter logic.
>
> -Peff
^ permalink raw reply [flat|nested] 115+ messages in thread
* [PATCH v3 11/23] ref-filter: rename field in ref_array_item stuct
2018-02-12 8:08 ` [PATCH v3 01/23] ref-filter: get rid of goto Olga Telezhnaya
` (14 preceding siblings ...)
2018-02-12 8:08 ` [PATCH v3 08/23] ref-filter: reuse parse_ref_filter_atom() Olga Telezhnaya
@ 2018-02-12 8:08 ` Olga Telezhnaya
2018-02-12 8:08 ` [PATCH v3 13/23] ref-filter: get rid of mark_atom_in_object_info() Olga Telezhnaya
` (6 subsequent siblings)
22 siblings, 0 replies; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-12 8:08 UTC (permalink / raw)
To: git
Rename objectname field to oid in struct ref_array_item.
Next commit will add objectname field that will contain
string representation of object id.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
builtin/cat-file.c | 4 ++--
ref-filter.c | 10 +++++-----
ref-filter.h | 2 +-
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 5b7bc34f1ec6d..0c362828ad81e 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -186,7 +186,7 @@ static void expand_atom(struct strbuf *sb, const char *atom, int len,
struct ref_array_item *item)
{
if (is_atom("objectname", atom, len))
- strbuf_addstr(sb, oid_to_hex(&item->objectname));
+ strbuf_addstr(sb, oid_to_hex(&item->oid));
else if (is_atom("objecttype", atom, len))
strbuf_addstr(sb, typename(item->type));
else if (is_atom("objectsize", atom, len))
@@ -294,7 +294,7 @@ static void batch_object_write(const char *obj_name, struct batch_options *opt,
return;
}
- item.objectname = data->oid;
+ item.oid = data->oid;
item.type = data->type;
item.size = data->size;
item.disk_size = data->disk_size;
diff --git a/ref-filter.c b/ref-filter.c
index 4acd391b5dfac..d09ec1bde6d54 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1489,7 +1489,7 @@ int populate_value(struct ref_array_item *ref)
v->s = xstrdup(buf + 1);
}
continue;
- } else if (!deref && grab_objectname(name, ref->objectname.hash, v, atom)) {
+ } else if (!deref && grab_objectname(name, ref->oid.hash, v, atom)) {
continue;
} else if (!strcmp(name, "HEAD")) {
if (atom->u.head && !strcmp(ref->refname, atom->u.head))
@@ -1534,13 +1534,13 @@ int populate_value(struct ref_array_item *ref)
if (used_atom_cnt <= i)
return 0;
- buf = get_obj(&ref->objectname, &obj, &size, &eaten);
+ buf = get_obj(&ref->oid, &obj, &size, &eaten);
if (!buf)
die(_("missing object %s for %s"),
- oid_to_hex(&ref->objectname), ref->refname);
+ oid_to_hex(&ref->oid), ref->refname);
if (!obj)
die(_("parse_object_buffer failed on %s for %s"),
- oid_to_hex(&ref->objectname), ref->refname);
+ oid_to_hex(&ref->oid), ref->refname);
grab_values(ref->value, 0, obj, buf, size);
if (!eaten)
@@ -1890,7 +1890,7 @@ static struct ref_array_item *new_ref_array_item(const char *refname,
{
struct ref_array_item *ref;
FLEX_ALLOC_STR(ref, refname, refname);
- hashcpy(ref->objectname.hash, objectname);
+ hashcpy(ref->oid.hash, objectname);
ref->flag = flag;
return ref;
diff --git a/ref-filter.h b/ref-filter.h
index e16ea2a990119..87b026b8b76d0 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -34,7 +34,7 @@ struct ref_sorting {
};
struct ref_array_item {
- struct object_id objectname;
+ struct object_id oid;
int flag;
unsigned int kind;
const char *symref;
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* [PATCH v3 13/23] ref-filter: get rid of mark_atom_in_object_info()
2018-02-12 8:08 ` [PATCH v3 01/23] ref-filter: get rid of goto Olga Telezhnaya
` (15 preceding siblings ...)
2018-02-12 8:08 ` [PATCH v3 11/23] ref-filter: rename field in ref_array_item stuct Olga Telezhnaya
@ 2018-02-12 8:08 ` Olga Telezhnaya
2018-02-15 5:45 ` Jeff King
2018-02-12 8:08 ` [PATCH v3 15/23] cat-file: move skip_object_info into ref-filter Olga Telezhnaya
` (5 subsequent siblings)
22 siblings, 1 reply; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-12 8:08 UTC (permalink / raw)
To: git
Remove mark_atom_in_object_info() and create same logic
in terms of ref-filter style.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
ref-filter.c | 45 +++++++++++++++++++++------------------------
1 file changed, 21 insertions(+), 24 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 3f92a27d98b6c..34a54db168265 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -255,13 +255,29 @@ static void objectname_atom_parser(const struct ref_format *format, struct used_
static void objectsize_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)
{
if (!arg)
- ; /* default to normal object size */
+ cat_file_info->info.sizep = &cat_file_info->size;
else if (!strcmp(arg, "disk"))
cat_file_info->info.disk_sizep = &cat_file_info->disk_size;
else
die(_("urecognized %%(objectsize) argument: %s"), arg);
}
+static void objecttype_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)
+{
+ if (!arg)
+ cat_file_info->info.typep = &cat_file_info->type;
+ else
+ die(_("urecognized %%(objecttype) argument: %s"), arg);
+}
+
+static void deltabase_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)
+{
+ if (!arg)
+ cat_file_info->info.delta_base_sha1 = cat_file_info->delta_base_oid.hash;
+ else
+ die(_("urecognized %%(deltabase) argument: %s"), arg);
+}
+
static void refname_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)
{
refname_atom_parser_internal(&atom->u.refname, arg, atom->name);
@@ -384,10 +400,10 @@ static struct valid_atom {
static struct valid_atom valid_cat_file_atom[] = {
{ "objectname" },
- { "objecttype" },
+ { "objecttype", FIELD_STR, objecttype_atom_parser },
{ "objectsize", FIELD_ULONG, objectsize_atom_parser },
{ "rest" },
- { "deltabase" },
+ { "deltabase", FIELD_STR, deltabase_atom_parser },
};
#define REF_FORMATTING_STATE_INIT { 0, NULL }
@@ -411,25 +427,6 @@ struct atom_value {
struct used_atom *atom;
};
-static int is_atom(const char *atom, const char *s, int slen)
-{
- int alen = strlen(atom);
- return alen == slen && !memcmp(atom, s, alen);
-}
-
-static void mark_atom_in_object_info(const char *atom, int len,
- struct expand_data *data)
-{
- if (is_atom("objecttype", atom, len))
- data->info.typep = &data->type;
- else if (is_atom("objectsize", atom, len))
- data->info.sizep = &data->size;
- else if (is_atom("rest", atom, len))
- data->split_on_whitespace = 1;
- else if (is_atom("deltabase", atom, len))
- data->info.delta_base_sha1 = data->delta_base_oid.hash;
-}
-
/*
* Used to parse format string and sort specifiers
*/
@@ -496,8 +493,8 @@ static int parse_ref_filter_atom(const struct ref_format *format,
need_tagged = 1;
if (!strcmp(valid_atom[i].name, "symref"))
need_symref = 1;
- if (cat_file_info)
- mark_atom_in_object_info(atom, atom_len, cat_file_info);
+ if (cat_file_info && !strcmp(valid_atom[i].name, "rest"))
+ cat_file_info->split_on_whitespace = 1;
return at;
}
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* Re: [PATCH v3 13/23] ref-filter: get rid of mark_atom_in_object_info()
2018-02-12 8:08 ` [PATCH v3 13/23] ref-filter: get rid of mark_atom_in_object_info() Olga Telezhnaya
@ 2018-02-15 5:45 ` Jeff King
2018-02-15 10:19 ` Оля Тележная
0 siblings, 1 reply; 115+ messages in thread
From: Jeff King @ 2018-02-15 5:45 UTC (permalink / raw)
To: Olga Telezhnaya; +Cc: git
On Mon, Feb 12, 2018 at 08:08:54AM +0000, Olga Telezhnaya wrote:
> Remove mark_atom_in_object_info() and create same logic
> in terms of ref-filter style.
This one is definitely a step in the right direction. In fact, this is
what I would have expected to see in the beginning. It seems like this
first half of the series really would benefit from being squashed into a
few commits. I.e., I'd have expected to see the whole series looking
something like:
- preparatory cleanup of ref-filter
- teach ref-filter any new atoms (or atom options) that cat-file knows
about but it doesn't
- convert cat-file to use ref-filter
Most of what I've seen so far is basically that second step, but strung
out along a bunch of commits. Can we compact it into a few commits that
all make clear forward progress (by using "rebase -i" with "squash")?
-Peff
^ permalink raw reply [flat|nested] 115+ messages in thread* Re: [PATCH v3 13/23] ref-filter: get rid of mark_atom_in_object_info()
2018-02-15 5:45 ` Jeff King
@ 2018-02-15 10:19 ` Оля Тележная
0 siblings, 0 replies; 115+ messages in thread
From: Оля Тележная @ 2018-02-15 10:19 UTC (permalink / raw)
To: Jeff King; +Cc: git
2018-02-15 8:45 GMT+03:00 Jeff King <peff@peff.net>:
> On Mon, Feb 12, 2018 at 08:08:54AM +0000, Olga Telezhnaya wrote:
>
>> Remove mark_atom_in_object_info() and create same logic
>> in terms of ref-filter style.
>
> This one is definitely a step in the right direction. In fact, this is
> what I would have expected to see in the beginning. It seems like this
> first half of the series really would benefit from being squashed into a
> few commits. I.e., I'd have expected to see the whole series looking
> something like:
>
> - preparatory cleanup of ref-filter
>
> - teach ref-filter any new atoms (or atom options) that cat-file knows
> about but it doesn't
>
> - convert cat-file to use ref-filter
>
> Most of what I've seen so far is basically that second step, but strung
> out along a bunch of commits. Can we compact it into a few commits that
> all make clear forward progress (by using "rebase -i" with "squash")?
I am afraid that I don't really see any 100% improvements by squashing
some of the commits. I tried to do that several times and I squashed
some of them, but now for me it looks like nothing else could be
squashed so that commits still look close to making atomic changes (I
am sure that it's super important).
>
> -Peff
^ permalink raw reply [flat|nested] 115+ messages in thread
* [PATCH v3 15/23] cat-file: move skip_object_info into ref-filter
2018-02-12 8:08 ` [PATCH v3 01/23] ref-filter: get rid of goto Olga Telezhnaya
` (16 preceding siblings ...)
2018-02-12 8:08 ` [PATCH v3 13/23] ref-filter: get rid of mark_atom_in_object_info() Olga Telezhnaya
@ 2018-02-12 8:08 ` Olga Telezhnaya
2018-02-15 5:51 ` Jeff King
2018-02-12 8:08 ` [PATCH v3 21/23] for-each-ref: tests for new atoms added Olga Telezhnaya
` (4 subsequent siblings)
22 siblings, 1 reply; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-12 8:08 UTC (permalink / raw)
To: git
Move logic related to skip_object_info into ref-filter,
so that cat-file does not use that field at all.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
builtin/cat-file.c | 7 +------
ref-filter.c | 5 +++++
ref-filter.h | 1 +
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 3a49b55a1cc2e..582679f3dca2c 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -393,14 +393,9 @@ static int batch_objects(struct batch_options *opt)
*/
memset(&data, 0, sizeof(data));
opt->format.cat_file_data = &data;
+ opt->format.all_objects = opt->all_objects;
verify_ref_format(&opt->format);
- if (opt->all_objects) {
- struct object_info empty = OBJECT_INFO_INIT;
- if (!memcmp(&data.info, &empty, sizeof(empty)))
- data.skip_object_info = 1;
- }
-
/*
* If we are printing out the object, then always fill in the type,
* since we will want to decide whether or not to stream.
diff --git a/ref-filter.c b/ref-filter.c
index 4adeea6aad0da..104cd6aef0102 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -775,6 +775,11 @@ int verify_ref_format(struct ref_format *format)
}
if (format->need_color_reset_at_eol && !want_color(format->use_color))
format->need_color_reset_at_eol = 0;
+ if (cat_file_info && format->all_objects) {
+ struct object_info empty = OBJECT_INFO_INIT;
+ if (!memcmp(&cat_file_info->info, &empty, sizeof(empty)))
+ cat_file_info->skip_object_info = 1;
+ }
return 0;
}
diff --git a/ref-filter.h b/ref-filter.h
index fffc443726e28..b1c668c12428b 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -118,6 +118,7 @@ struct ref_format {
* hopefully would be reduced later.
*/
struct expand_data *cat_file_data;
+ unsigned all_objects : 1;
};
#define REF_FORMAT_INIT { NULL, 0, -1 }
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* Re: [PATCH v3 15/23] cat-file: move skip_object_info into ref-filter
2018-02-12 8:08 ` [PATCH v3 15/23] cat-file: move skip_object_info into ref-filter Olga Telezhnaya
@ 2018-02-15 5:51 ` Jeff King
2018-02-15 10:27 ` Оля Тележная
0 siblings, 1 reply; 115+ messages in thread
From: Jeff King @ 2018-02-15 5:51 UTC (permalink / raw)
To: Olga Telezhnaya; +Cc: git
On Mon, Feb 12, 2018 at 08:08:54AM +0000, Olga Telezhnaya wrote:
> Move logic related to skip_object_info into ref-filter,
> so that cat-file does not use that field at all.
I think this is going the wrong way. ref-filter should always do as
little work as possible to fulfill the request. So it should skip the
object_info call whenever it can. And then any callers who want to make
sure that the object exists can do so (as long as the formatting code
tells them whether it looked up the object or not).
And then ref-filter doesn't have to know about this skip_object_info
flag at all.
-Peff
^ permalink raw reply [flat|nested] 115+ messages in thread
* Re: [PATCH v3 15/23] cat-file: move skip_object_info into ref-filter
2018-02-15 5:51 ` Jeff King
@ 2018-02-15 10:27 ` Оля Тележная
0 siblings, 0 replies; 115+ messages in thread
From: Оля Тележная @ 2018-02-15 10:27 UTC (permalink / raw)
To: Jeff King; +Cc: git
2018-02-15 8:51 GMT+03:00 Jeff King <peff@peff.net>:
> On Mon, Feb 12, 2018 at 08:08:54AM +0000, Olga Telezhnaya wrote:
>
>> Move logic related to skip_object_info into ref-filter,
>> so that cat-file does not use that field at all.
>
> I think this is going the wrong way. ref-filter should always do as
> little work as possible to fulfill the request. So it should skip the
> object_info call whenever it can. And then any callers who want to make
> sure that the object exists can do so (as long as the formatting code
> tells them whether it looked up the object or not).
>
> And then ref-filter doesn't have to know about this skip_object_info
> flag at all.
Your message looks contradictory to me.
I agree that ref-filter should do as least as it's possible, and that
is the main reason why I put this code there. Moreover, I think that
it's a good idea to implement that variable not only for cat-file, but
for all ref-filter callers. And I think that it's a task of ref-filter
to check whether the object exists or not (or - whether the ref is
valid or not). But I am not sure that I need to solve that moment in
current patch. It sounds like another separate task.
>
> -Peff
^ permalink raw reply [flat|nested] 115+ messages in thread
* [PATCH v3 21/23] for-each-ref: tests for new atoms added
2018-02-12 8:08 ` [PATCH v3 01/23] ref-filter: get rid of goto Olga Telezhnaya
` (17 preceding siblings ...)
2018-02-12 8:08 ` [PATCH v3 15/23] cat-file: move skip_object_info into ref-filter Olga Telezhnaya
@ 2018-02-12 8:08 ` Olga Telezhnaya
2018-02-15 5:57 ` Jeff King
2018-02-12 8:08 ` [PATCH v3 02/23] ref-filter: add return value to some functions Olga Telezhnaya
` (3 subsequent siblings)
22 siblings, 1 reply; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-12 8:08 UTC (permalink / raw)
To: git
Add tests for new formatting atoms: rest, deltabase, objectsize:disk.
rest means nothing and we expand it into empty string.
We need this atom for cat-file command.
Have plans to support deltabase and objectsize:disk further
(as it is done in cat-file), now also expand it to empty string.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
t/t6300-for-each-ref.sh | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index c128dfc579079..eee656a6abba9 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -316,6 +316,24 @@ test_expect_success 'exercise strftime with odd fields' '
test_cmp expected actual
'
+test_expect_success 'Check format %(objectsize:disk) gives empty output ' '
+ echo >expected &&
+ git for-each-ref --format="%(objectsize:disk)" refs/heads >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'Check format %(rest) gives empty output ' '
+ echo >expected &&
+ git for-each-ref --format="%(rest)" refs/heads >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'Check format %(deltabase) gives empty output ' '
+ echo >expected &&
+ git for-each-ref --format="%(deltabase)" refs/heads >actual &&
+ test_cmp expected actual
+'
+
cat >expected <<\EOF
refs/heads/master
refs/remotes/origin/master
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* Re: [PATCH v3 21/23] for-each-ref: tests for new atoms added
2018-02-12 8:08 ` [PATCH v3 21/23] for-each-ref: tests for new atoms added Olga Telezhnaya
@ 2018-02-15 5:57 ` Jeff King
2018-02-15 10:38 ` Оля Тележная
0 siblings, 1 reply; 115+ messages in thread
From: Jeff King @ 2018-02-15 5:57 UTC (permalink / raw)
To: Olga Telezhnaya; +Cc: git
On Mon, Feb 12, 2018 at 08:08:54AM +0000, Olga Telezhnaya wrote:
> Add tests for new formatting atoms: rest, deltabase, objectsize:disk.
> rest means nothing and we expand it into empty string.
> We need this atom for cat-file command.
> Have plans to support deltabase and objectsize:disk further
> (as it is done in cat-file), now also expand it to empty string.
I'm glad that you're adding tests, but I'm not sure it's a good idea to
add tests checking for the thing we know to be wrong. If anything, you
could be adding test_expect_failure looking for the _right_ thing, and
accept that it does not yet work.
-Peff
^ permalink raw reply [flat|nested] 115+ messages in thread
* Re: [PATCH v3 21/23] for-each-ref: tests for new atoms added
2018-02-15 5:57 ` Jeff King
@ 2018-02-15 10:38 ` Оля Тележная
0 siblings, 0 replies; 115+ messages in thread
From: Оля Тележная @ 2018-02-15 10:38 UTC (permalink / raw)
To: Jeff King; +Cc: git
2018-02-15 8:57 GMT+03:00 Jeff King <peff@peff.net>:
> On Mon, Feb 12, 2018 at 08:08:54AM +0000, Olga Telezhnaya wrote:
>
>> Add tests for new formatting atoms: rest, deltabase, objectsize:disk.
>> rest means nothing and we expand it into empty string.
>> We need this atom for cat-file command.
>> Have plans to support deltabase and objectsize:disk further
>> (as it is done in cat-file), now also expand it to empty string.
>
> I'm glad that you're adding tests, but I'm not sure it's a good idea to
> add tests checking for the thing we know to be wrong. If anything, you
> could be adding test_expect_failure looking for the _right_ thing, and
> accept that it does not yet work.
OK, I will try to fix that when other parts of the patch will look
good enough. I am not sure at this point that we will add my code to
the main version of the project, so these tests were written actually
for checking current functionality better before sending the code for
the review.
>
> -Peff
^ permalink raw reply [flat|nested] 115+ messages in thread
* [PATCH v3 02/23] ref-filter: add return value to some functions
2018-02-12 8:08 ` [PATCH v3 01/23] ref-filter: get rid of goto Olga Telezhnaya
` (18 preceding siblings ...)
2018-02-12 8:08 ` [PATCH v3 21/23] for-each-ref: tests for new atoms added Olga Telezhnaya
@ 2018-02-12 8:08 ` Olga Telezhnaya
2018-02-15 5:16 ` Jeff King
2018-02-12 8:08 ` [PATCH v3 07/23] cat-file: start migrating formatting to ref-filter Olga Telezhnaya
` (2 subsequent siblings)
22 siblings, 1 reply; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-12 8:08 UTC (permalink / raw)
To: git
Add return flag to format_ref_array_item(), show_ref_array_item(),
get_ref_array_info() and populate_value() for further using.
Need it to handle situations when item is broken but we can not invoke
die() because we are in batch mode and all items need to be processed.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
ref-filter.c | 37 ++++++++++++++++++++++++-------------
ref-filter.h | 14 ++++++++++----
2 files changed, 34 insertions(+), 17 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index d04295e33448e..9ed5e66066a7a 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1356,8 +1356,9 @@ static const char *get_refname(struct used_atom *atom, struct ref_array_item *re
/*
* Parse the object referred by ref, and grab needed value.
+ * Return 0 if everything was successful, -1 otherwise.
*/
-static void populate_value(struct ref_array_item *ref)
+static int populate_value(struct ref_array_item *ref)
{
void *buf;
struct object *obj;
@@ -1482,7 +1483,7 @@ static void populate_value(struct ref_array_item *ref)
}
}
if (used_atom_cnt <= i)
- return;
+ return 0;
buf = get_obj(&ref->objectname, &obj, &size, &eaten);
if (!buf)
@@ -1501,7 +1502,7 @@ static void populate_value(struct ref_array_item *ref)
* object, we are done.
*/
if (!need_tagged || (obj->type != OBJ_TAG))
- return;
+ return 0;
/*
* If it is a tag object, see if we use a value that derefs
@@ -1525,19 +1526,24 @@ static void populate_value(struct ref_array_item *ref)
grab_values(ref->value, 1, obj, buf, size);
if (!eaten)
free(buf);
+
+ return 0;
}
/*
* Given a ref, return the value for the atom. This lazily gets value
* out of the object by calling populate value.
+ * Return 0 if everything was successful, -1 otherwise.
*/
-static void get_ref_atom_value(struct ref_array_item *ref, int atom, struct atom_value **v)
+static int get_ref_atom_value(struct ref_array_item *ref, int atom, struct atom_value **v)
{
+ int retval = 0;
if (!ref->value) {
- populate_value(ref);
+ retval = populate_value(ref);
fill_missing_values(ref->value);
}
*v = &ref->value[atom];
+ return retval;
}
/*
@@ -2122,7 +2128,7 @@ static void append_literal(const char *cp, const char *ep, struct ref_formatting
}
}
-void format_ref_array_item(struct ref_array_item *info,
+int format_ref_array_item(struct ref_array_item *info,
const struct ref_format *format,
struct strbuf *final_buf)
{
@@ -2138,9 +2144,10 @@ void format_ref_array_item(struct ref_array_item *info,
ep = strchr(sp, ')');
if (cp < sp)
append_literal(cp, sp, &state);
- get_ref_atom_value(info,
- parse_ref_filter_atom(format, sp + 2, ep),
- &atomv);
+ if (get_ref_atom_value(info,
+ parse_ref_filter_atom(format, sp + 2, ep),
+ &atomv))
+ return -1;
atomv->handler(atomv, &state);
}
if (*cp) {
@@ -2156,17 +2163,21 @@ void format_ref_array_item(struct ref_array_item *info,
die(_("format: %%(end) atom missing"));
strbuf_addbuf(final_buf, &state.stack->output);
pop_stack_element(&state.stack);
+ return 0;
}
-void show_ref_array_item(struct ref_array_item *info,
+int show_ref_array_item(struct ref_array_item *info,
const struct ref_format *format)
{
struct strbuf final_buf = STRBUF_INIT;
+ int retval = format_ref_array_item(info, format, &final_buf);
- format_ref_array_item(info, format, &final_buf);
- fwrite(final_buf.buf, 1, final_buf.len, stdout);
+ if (!retval) {
+ fwrite(final_buf.buf, 1, final_buf.len, stdout);
+ putchar('\n');
+ }
strbuf_release(&final_buf);
- putchar('\n');
+ return retval;
}
void pretty_print_ref(const char *name, const unsigned char *sha1,
diff --git a/ref-filter.h b/ref-filter.h
index 0d98342b34319..b75c8ac45248e 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -109,12 +109,18 @@ void ref_array_clear(struct ref_array *array);
int verify_ref_format(struct ref_format *format);
/* Sort the given ref_array as per the ref_sorting provided */
void ref_array_sort(struct ref_sorting *sort, struct ref_array *array);
-/* Based on the given format and quote_style, fill the strbuf */
-void format_ref_array_item(struct ref_array_item *info,
+/*
+ * Based on the given format and quote_style, fill the strbuf.
+ * Return 0 if everything was successful, -1 otherwise (and strbuf remains empty)
+ */
+int format_ref_array_item(struct ref_array_item *info,
const struct ref_format *format,
struct strbuf *final_buf);
-/* Print the ref using the given format and quote_style */
-void show_ref_array_item(struct ref_array_item *info, const struct ref_format *format);
+/*
+ * Print the ref using the given format and quote_style.
+ * Return 0 if everything was successful, -1 otherwise.
+ */
+int show_ref_array_item(struct ref_array_item *info, const struct ref_format *format);
/* Parse a single sort specifier and add it to the list */
void parse_ref_sorting(struct ref_sorting **sorting_tail, const char *atom);
/* Callback function for parsing the sort option */
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* Re: [PATCH v3 02/23] ref-filter: add return value to some functions
2018-02-12 8:08 ` [PATCH v3 02/23] ref-filter: add return value to some functions Olga Telezhnaya
@ 2018-02-15 5:16 ` Jeff King
2018-02-15 9:59 ` Оля Тележная
0 siblings, 1 reply; 115+ messages in thread
From: Jeff King @ 2018-02-15 5:16 UTC (permalink / raw)
To: Olga Telezhnaya; +Cc: git
On Mon, Feb 12, 2018 at 08:08:54AM +0000, Olga Telezhnaya wrote:
> Add return flag to format_ref_array_item(), show_ref_array_item(),
> get_ref_array_info() and populate_value() for further using.
> Need it to handle situations when item is broken but we can not invoke
> die() because we are in batch mode and all items need to be processed.
OK. The source of these errors would eventually be calls in
populate_value(), but we don't flag any errors there yet (well, we do,
but they all end up in die() for now). So I'd expect to see later in the
series those die() calls converted to errors (I haven't looked further
yet; just making a note to myself).
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1356,8 +1356,9 @@ static const char *get_refname(struct used_atom *atom, struct ref_array_item *re
>
> /*
> * Parse the object referred by ref, and grab needed value.
> + * Return 0 if everything was successful, -1 otherwise.
> */
We discussed off-list the concept that the caller may want to know one
of three outcomes:
- we completed the request, having accessed the object
- we completed the request, but it didn't require accessing any
objects
- an error occurred accessing the object
Since callers like "cat-file" would need to check has_sha1_file()
manually in the second case. Should this return value actually be an
enum, which would make it easier to convert later to a tri-state?
> -static void populate_value(struct ref_array_item *ref)
> +static int populate_value(struct ref_array_item *ref)
> {
> void *buf;
> struct object *obj;
> @@ -1482,7 +1483,7 @@ static void populate_value(struct ref_array_item *ref)
> }
> }
> if (used_atom_cnt <= i)
> - return;
> + return 0;
Most of these conversions are obviously correct, because they just turn
a void return into one with a value. But this one is trickier:
> @@ -2138,9 +2144,10 @@ void format_ref_array_item(struct ref_array_item *info,
> ep = strchr(sp, ')');
> if (cp < sp)
> append_literal(cp, sp, &state);
> - get_ref_atom_value(info,
> - parse_ref_filter_atom(format, sp + 2, ep),
> - &atomv);
> + if (get_ref_atom_value(info,
> + parse_ref_filter_atom(format, sp + 2, ep),
> + &atomv))
> + return -1;
> atomv->handler(atomv, &state);
> }
since it affects the control flow. Might we be skipping any necessary
cleanup in the function if we see an error?
It looks like we may have called push_stack_element(), but we'd never
get to the end of the function where we call pop_stack_element(),
causing us to leak.
-Peff
^ permalink raw reply [flat|nested] 115+ messages in thread* Re: [PATCH v3 02/23] ref-filter: add return value to some functions
2018-02-15 5:16 ` Jeff King
@ 2018-02-15 9:59 ` Оля Тележная
0 siblings, 0 replies; 115+ messages in thread
From: Оля Тележная @ 2018-02-15 9:59 UTC (permalink / raw)
To: Jeff King; +Cc: git
2018-02-15 8:16 GMT+03:00 Jeff King <peff@peff.net>:
> On Mon, Feb 12, 2018 at 08:08:54AM +0000, Olga Telezhnaya wrote:
>
>> Add return flag to format_ref_array_item(), show_ref_array_item(),
>> get_ref_array_info() and populate_value() for further using.
>> Need it to handle situations when item is broken but we can not invoke
>> die() because we are in batch mode and all items need to be processed.
>
> OK. The source of these errors would eventually be calls in
> populate_value(), but we don't flag any errors there yet (well, we do,
> but they all end up in die() for now). So I'd expect to see later in the
> series those die() calls converted to errors (I haven't looked further
> yet; just making a note to myself).
>
>> --- a/ref-filter.c
>> +++ b/ref-filter.c
>> @@ -1356,8 +1356,9 @@ static const char *get_refname(struct used_atom *atom, struct ref_array_item *re
>>
>> /*
>> * Parse the object referred by ref, and grab needed value.
>> + * Return 0 if everything was successful, -1 otherwise.
>> */
>
> We discussed off-list the concept that the caller may want to know one
> of three outcomes:
>
> - we completed the request, having accessed the object
> - we completed the request, but it didn't require accessing any
> objects
> - an error occurred accessing the object
>
> Since callers like "cat-file" would need to check has_sha1_file()
> manually in the second case. Should this return value actually be an
> enum, which would make it easier to convert later to a tri-state?
I decided not to implement this particular scenario because all other
callers are waiting that everything will be printed inside ref-filter.
We just add support for cat-file there. I don't think that I need to
re-think all printing process and move printing logic to all other
callers so that cat-file will behave fine. In my opinion, in the final
version cat-file must accept all ref-filter logic parts and adapt to
them.
>
>> -static void populate_value(struct ref_array_item *ref)
>> +static int populate_value(struct ref_array_item *ref)
>> {
>> void *buf;
>> struct object *obj;
>> @@ -1482,7 +1483,7 @@ static void populate_value(struct ref_array_item *ref)
>> }
>> }
>> if (used_atom_cnt <= i)
>> - return;
>> + return 0;
>
> Most of these conversions are obviously correct, because they just turn
> a void return into one with a value. But this one is trickier:
>
>> @@ -2138,9 +2144,10 @@ void format_ref_array_item(struct ref_array_item *info,
>> ep = strchr(sp, ')');
>> if (cp < sp)
>> append_literal(cp, sp, &state);
>> - get_ref_atom_value(info,
>> - parse_ref_filter_atom(format, sp + 2, ep),
>> - &atomv);
>> + if (get_ref_atom_value(info,
>> + parse_ref_filter_atom(format, sp + 2, ep),
>> + &atomv))
>> + return -1;
>> atomv->handler(atomv, &state);
>> }
>
> since it affects the control flow. Might we be skipping any necessary
> cleanup in the function if we see an error?
>
> It looks like we may have called push_stack_element(), but we'd never
> get to the end of the function where we call pop_stack_element(),
> causing us to leak.
Agree, I will fix this.
>
> -Peff
^ permalink raw reply [flat|nested] 115+ messages in thread
* [PATCH v3 07/23] cat-file: start migrating formatting to ref-filter
2018-02-12 8:08 ` [PATCH v3 01/23] ref-filter: get rid of goto Olga Telezhnaya
` (19 preceding siblings ...)
2018-02-12 8:08 ` [PATCH v3 02/23] ref-filter: add return value to some functions Olga Telezhnaya
@ 2018-02-12 8:08 ` Olga Telezhnaya
2018-02-15 5:32 ` Jeff King
2018-02-12 8:08 ` [PATCH v3 05/23] cat-file: move struct expand_data into ref-filter Olga Telezhnaya
2018-02-15 5:08 ` [PATCH v3 01/23] ref-filter: get rid of goto Jeff King
22 siblings, 1 reply; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-12 8:08 UTC (permalink / raw)
To: git
Move mark_atom_in_object_info() from cat-file to ref-filter and
start using it in verify_ref_format().
It also means that we start reusing verify_ref_format() in cat-file.
Start from simple moving of mark_atom_in_object_info(),
it would be removed later by integrating all needed processes into
ref-filter logic.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
builtin/cat-file.c | 35 +++++------------------------------
ref-filter.c | 41 ++++++++++++++++++++++++++++++++++++-----
ref-filter.h | 12 ++++++------
3 files changed, 47 insertions(+), 41 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index edb04a96d9bd3..67e7790d2f319 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -182,25 +182,6 @@ static int is_atom(const char *atom, const char *s, int slen)
return alen == slen && !memcmp(atom, s, alen);
}
-static void mark_atom_in_object_info(const char *atom, int len,
- struct expand_data *data)
-{
- if (is_atom("objectname", atom, len))
- ; /* do nothing */
- else if (is_atom("objecttype", atom, len))
- data->info.typep = &data->type;
- else if (is_atom("objectsize", atom, len))
- data->info.sizep = &data->size;
- else if (is_atom("objectsize:disk", atom, len))
- data->info.disk_sizep = &data->disk_size;
- else if (is_atom("rest", atom, len))
- data->split_on_whitespace = 1;
- else if (is_atom("deltabase", atom, len))
- data->info.delta_base_sha1 = data->delta_base_oid.hash;
- else
- die("unknown format element: %.*s", len, atom);
-}
-
static void expand_atom(struct strbuf *sb, const char *atom, int len,
struct expand_data *data)
{
@@ -230,11 +211,7 @@ static size_t expand_format(struct strbuf *sb, const char *start, void *vdata)
if (!end)
die("format element '%s' does not end in ')'", start);
- if (data->mark_query)
- mark_atom_in_object_info(start + 1, end - start - 1, data);
- else
- expand_atom(sb, start + 1, end - start - 1, data);
-
+ expand_atom(sb, start + 1, end - start - 1, data);
return end - start + 1;
}
@@ -413,14 +390,12 @@ static int batch_objects(struct batch_options *opt)
opt->format.format = "%(objectname) %(objecttype) %(objectsize)";
/*
- * Expand once with our special mark_query flag, which will prime the
- * object_info to be handed to sha1_object_info_extended for each
- * object.
+ * Call verify_ref_format to prepare object_info to be handed to
+ * sha1_object_info_extended for each object.
*/
memset(&data, 0, sizeof(data));
- data.mark_query = 1;
- strbuf_expand(&buf, opt->format.format, expand_format, &data);
- data.mark_query = 0;
+ opt->format.cat_file_data = &data;
+ verify_ref_format(&opt->format);
if (opt->cmdmode)
data.split_on_whitespace = 1;
diff --git a/ref-filter.c b/ref-filter.c
index 5e7ed0f338490..ff87e632f463c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -392,6 +392,31 @@ struct atom_value {
struct used_atom *atom;
};
+static int is_atom(const char *atom, const char *s, int slen)
+{
+ int alen = strlen(atom);
+ return alen == slen && !memcmp(atom, s, alen);
+}
+
+static void mark_atom_in_object_info(const char *atom, int len,
+ struct expand_data *data)
+{
+ if (is_atom("objectname", atom, len))
+ ; /* do nothing */
+ else if (is_atom("objecttype", atom, len))
+ data->info.typep = &data->type;
+ else if (is_atom("objectsize", atom, len))
+ data->info.sizep = &data->size;
+ else if (is_atom("objectsize:disk", atom, len))
+ data->info.disk_sizep = &data->disk_size;
+ else if (is_atom("rest", atom, len))
+ data->split_on_whitespace = 1;
+ else if (is_atom("deltabase", atom, len))
+ data->info.delta_base_sha1 = data->delta_base_oid.hash;
+ else
+ die("unknown format element: %.*s", len, atom);
+}
+
/*
* Used to parse format string and sort specifiers
*/
@@ -709,12 +734,18 @@ int verify_ref_format(struct ref_format *format)
if (!ep)
return error(_("malformed format string %s"), sp);
/* sp points at "%(" and ep points at the closing ")" */
- at = parse_ref_filter_atom(format, valid_atom,
- ARRAY_SIZE(valid_atom), sp + 2, ep);
- cp = ep + 1;
- if (skip_prefix(used_atom[at].name, "color:", &color))
- format->need_color_reset_at_eol = !!strcmp(color, "reset");
+ if (format->cat_file_data)
+ mark_atom_in_object_info(sp + 2, ep - sp - 2,
+ format->cat_file_data);
+ else {
+ at = parse_ref_filter_atom(format, valid_atom,
+ ARRAY_SIZE(valid_atom), sp + 2, ep);
+ if (skip_prefix(used_atom[at].name, "color:", &color))
+ format->need_color_reset_at_eol = !!strcmp(color, "reset");
+ }
+
+ cp = ep + 1;
}
if (format->need_color_reset_at_eol && !want_color(format->use_color))
format->need_color_reset_at_eol = 0;
diff --git a/ref-filter.h b/ref-filter.h
index 17f2ac24d2739..52e07dbe6864a 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -80,12 +80,6 @@ struct expand_data {
const char *rest;
struct object_id delta_base_oid;
- /*
- * If mark_query is true, we do not expand anything, but rather
- * just mark the object_info with items we wish to query.
- */
- int mark_query;
-
/*
* Whether to split the input on whitespace before feeding it to
* get_sha1; this is decided during the mark_query phase based on
@@ -119,6 +113,12 @@ struct ref_format {
/* Internal state to ref-filter */
int need_color_reset_at_eol;
+
+ /*
+ * Helps to move all formatting logic from cat-file to ref-filter,
+ * hopefully would be reduced later.
+ */
+ struct expand_data *cat_file_data;
};
#define REF_FORMAT_INIT { NULL, 0, -1 }
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* Re: [PATCH v3 07/23] cat-file: start migrating formatting to ref-filter
2018-02-12 8:08 ` [PATCH v3 07/23] cat-file: start migrating formatting to ref-filter Olga Telezhnaya
@ 2018-02-15 5:32 ` Jeff King
0 siblings, 0 replies; 115+ messages in thread
From: Jeff King @ 2018-02-15 5:32 UTC (permalink / raw)
To: Olga Telezhnaya; +Cc: git
On Mon, Feb 12, 2018 at 08:08:54AM +0000, Olga Telezhnaya wrote:
> Move mark_atom_in_object_info() from cat-file to ref-filter and
> start using it in verify_ref_format().
> It also means that we start reusing verify_ref_format() in cat-file.
>
> Start from simple moving of mark_atom_in_object_info(),
> it would be removed later by integrating all needed processes into
> ref-filter logic.
I see where you're going here, but it feels like there's a lot of
boilerplate to move this thing whole, when we plan to get rid of it in
the long run. After all, we _already_ support most of these placeholders
in ref-filter. I'll keep reading...
-Peff
^ permalink raw reply [flat|nested] 115+ messages in thread
* [PATCH v3 05/23] cat-file: move struct expand_data into ref-filter
2018-02-12 8:08 ` [PATCH v3 01/23] ref-filter: get rid of goto Olga Telezhnaya
` (20 preceding siblings ...)
2018-02-12 8:08 ` [PATCH v3 07/23] cat-file: start migrating formatting to ref-filter Olga Telezhnaya
@ 2018-02-12 8:08 ` Olga Telezhnaya
2018-02-15 5:26 ` Jeff King
2018-02-15 5:08 ` [PATCH v3 01/23] ref-filter: get rid of goto Jeff King
22 siblings, 1 reply; 115+ messages in thread
From: Olga Telezhnaya @ 2018-02-12 8:08 UTC (permalink / raw)
To: git
Need that for further reusing of formatting logic in cat-file.
Have plans to get rid of using expand_data in cat-file at all,
and use it only in ref-filter for collecting, formatting and printing
needed data.
Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored by: Jeff King <peff@peff.net>
---
builtin/cat-file.c | 36 ------------------------------------
ref-filter.h | 36 ++++++++++++++++++++++++++++++++++++
2 files changed, 36 insertions(+), 36 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 98fc5ec069a49..37d6096d201b5 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -176,42 +176,6 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
return 0;
}
-struct expand_data {
- struct object_id oid;
- enum object_type type;
- unsigned long size;
- off_t disk_size;
- const char *rest;
- struct object_id delta_base_oid;
-
- /*
- * If mark_query is true, we do not expand anything, but rather
- * just mark the object_info with items we wish to query.
- */
- int mark_query;
-
- /*
- * Whether to split the input on whitespace before feeding it to
- * get_sha1; this is decided during the mark_query phase based on
- * whether we have a %(rest) token in our format.
- */
- int split_on_whitespace;
-
- /*
- * After a mark_query run, this object_info is set up to be
- * passed to sha1_object_info_extended. It will point to the data
- * elements above, so you can retrieve the response from there.
- */
- struct object_info info;
-
- /*
- * This flag will be true if the requested batch format and options
- * don't require us to call sha1_object_info, which can then be
- * optimized out.
- */
- unsigned skip_object_info : 1;
-};
-
static int is_atom(const char *atom, const char *s, int slen)
{
int alen = strlen(atom);
diff --git a/ref-filter.h b/ref-filter.h
index b75c8ac45248e..17f2ac24d2739 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -72,6 +72,42 @@ struct ref_filter {
verbose;
};
+struct expand_data {
+ struct object_id oid;
+ enum object_type type;
+ unsigned long size;
+ off_t disk_size;
+ const char *rest;
+ struct object_id delta_base_oid;
+
+ /*
+ * If mark_query is true, we do not expand anything, but rather
+ * just mark the object_info with items we wish to query.
+ */
+ int mark_query;
+
+ /*
+ * Whether to split the input on whitespace before feeding it to
+ * get_sha1; this is decided during the mark_query phase based on
+ * whether we have a %(rest) token in our format.
+ */
+ int split_on_whitespace;
+
+ /*
+ * After a mark_query run, this object_info is set up to be
+ * passed to sha1_object_info_extended. It will point to the data
+ * elements above, so you can retrieve the response from there.
+ */
+ struct object_info info;
+
+ /*
+ * This flag will be true if the requested batch format and options
+ * don't require us to call sha1_object_info, which can then be
+ * optimized out.
+ */
+ unsigned skip_object_info : 1;
+};
+
struct ref_format {
/*
* Set these to define the format; make sure you call
--
https://github.com/git/git/pull/452
^ permalink raw reply related [flat|nested] 115+ messages in thread* Re: [PATCH v3 01/23] ref-filter: get rid of goto
2018-02-12 8:08 ` [PATCH v3 01/23] ref-filter: get rid of goto Olga Telezhnaya
` (21 preceding siblings ...)
2018-02-12 8:08 ` [PATCH v3 05/23] cat-file: move struct expand_data into ref-filter Olga Telezhnaya
@ 2018-02-15 5:08 ` Jeff King
22 siblings, 0 replies; 115+ messages in thread
From: Jeff King @ 2018-02-15 5:08 UTC (permalink / raw)
To: Olga Telezhnaya; +Cc: git
On Mon, Feb 12, 2018 at 08:08:54AM +0000, Olga Telezhnaya wrote:
> Get rid of goto command in ref-filter for better readability.
>
> Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored by: Jeff King <peff@peff.net>
OK, makes sense, and the patch looks correct.
-Peff
^ permalink raw reply [flat|nested] 115+ messages in thread