ref-filter: fix %(symref) for pretty_print_ref()
cover ref-filter: fix %(symref) for pretty_print_ref()
Because pretty_print_ref() currently has no ref_item flag, but the implementation of %(symref) need this flag, so git tag --verify or git verify-tag cannot get the output of %(symref). Therefore, %(symref) will be fixed in this patch series.
This bug was discovered by Peff here:
https://lore.kernel.org/git/YUO63qy2%2F5wibY4%[email protected]/
on the other hand, git tag --verify --format="%(refname)" or git verify-tag --forrmat="%(refname)" does not show the fullref name, but seems to be similar to using %(refname:lstrip=2).
And git verify-tag --format="verify: %(refname)" c06b72d02` will output:
"verify: c06b72d02". They are wrong features, but because some scripts are using them, so our current approach is to keep them.
ZheNing Hu (4):
refs: let repo_dwim_ref() learn get symref itself and ref flags
ref-filter: provide ref_flags to pretty_print_ref()
verify_tag: use repo_dwim_ref() to get ref fullname and ref_flags
ref-filter: let tag verify use %(refname:lstrip=2) by default
builtin/tag.c | 16 ++++++++------- builtin/verify-tag.c | 9 ++++++++- object-name.c | 6 +++--- ref-filter.c | 47 +++++++++++++++++++++++++++++++++---------- ref-filter.h | 4 +++- refs.c | 15 +++++++++----- refs.h | 8 +++++--- t/t7030-verify-tag.sh | 23 +++++++++++++++++++++ upload-pack.c | 2 +- 9 files changed, 98 insertions(+), 32 deletions(-)
base-commit: 8b7c11b8668b4e774f81a9f0b4c30144b818f1d1
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1042%2Fadlternative%2Fpretty-print-ref-symref-fix-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1042/adlternative/pretty-print-ref-symref-fix-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1042
Signature · 1 line
gitgitgadget
PATCH 1/4 refs: let repo_dwim_ref() learn get symref itself and ref flags
From: ZheNing Hu <[email protected]>
expand_ref() can use to resolve a ref to its fullname, A symref will also be resolved to the fullref name it refers to. But sometimes we want get symref itself. So add a need_symref parameter to repo_dwim_ref() and expand_ref(), which can help us get symref its fullref name. At the same time, we add ref_flags parameter to expand_ref() and repo_dwim_ref(), when it is set, it can get the ref's flag from refs_resolve_ref_unsafe(), which can help us provide ref's flags for interfaces like pretty_print_ref() later.
--- object-name.c | 6 +++--- refs.c | 15 ++++++++++----- refs.h | 8 +++++--- upload-pack.c | 2 +- 4 files changed, 19 insertions(+), 12 deletions(-)
object-name.c +3 -3
@@ -803,7 +803,7 @@ static int get_oid_basic(struct repository *r, const char *str, int len,if (len == r->hash_algo->hexsz && !get_oid_hex (str, oid)) {if (warn_ambiguous_refs && warn_on_object_refname_ambiguity) { - refs_found =repo_dwim_ref (r, str, len, &tmp_oid, &real_ref,0 ); + refs_found =repo_dwim_ref (r, str, len, &tmp_oid, &real_ref,0 ,0 ,0 );if (refs_found >0 ) {warning (warn_msg, len, str);if (advice_enabled (ADVICE_OBJECT_NAME_WARNING ))
@@ -854,11 +854,11 @@ static int get_oid_basic(struct repository *r, const char *str, int len,if (!len && reflog_len)/* allow "@{...}" to mean the current branch reflog */ - refs_found =repo_dwim_ref (r,"HEAD" ,4 , oid, &real_ref,0 ); + refs_found =repo_dwim_ref (r,"HEAD" ,4 , oid, &real_ref,0 ,0 ,0 );else if (reflog_len) refs_found =repo_dwim_log (r, str, len, oid, &real_ref);else - refs_found =repo_dwim_ref (r, str, len, oid, &real_ref,0 ); + refs_found =repo_dwim_ref (r, str, len, oid, &real_ref,0 ,0 ,0 );if (!refs_found)return -1 ;
refs.c +10 -5
@@ -637,17 +637,19 @@ static char *substitute_branch_name(struct repository *r, }int repo_dwim_ref (struct repository *r,const char *str,int len, -struct object_id *oid,char **ref,int nonfatal_dangling_mark) +struct object_id *oid,char **ref,int nonfatal_dangling_mark, +int *ref_flags,int need_symref) {char *last_branch =substitute_branch_name (r, &str, &len, nonfatal_dangling_mark); -int refs_found =expand_ref (r, str, len, oid, ref); +int refs_found =expand_ref (r, str, len, oid, ref, ref_flags, need_symref);free (last_branch);return refs_found; }int expand_ref (struct repository *repo,const char *str,int len, -struct object_id *oid,char **ref) +struct object_id *oid,char **ref,int *ref_flags, +int need_symref) {const char **p, *r;int refs_found =0 ;
@@ -666,8 +668,11 @@ int expand_ref(struct repository *repo, const char *str, int len, fullref.buf,RESOLVE_REF_READING , this_result, &flag);if (r) { -if (!refs_found++) - *ref =xstrdup (r); +if (!refs_found++) { + *ref =xstrdup (need_symref ? fullref.buf : r); +if (ref_flags) + *ref_flags = flag; + }if (!warn_ambiguous_refs)break ; }else if ((flag &REF_ISSYMREF ) &&strcmp (fullref.buf,"HEAD" )) {
refs.h +5 -3
@@ -152,15 +152,17 @@ int refname_match(const char *abbrev_name, const char *full_name);struct strvec ;void expand_ref_prefix (struct strvec *prefixes,const char *prefix); -int expand_ref (struct repository *r,const char *str,int len,struct object_id *oid,char **ref); +int expand_ref (struct repository *r,const char *str,int len,struct object_id *oid,char **ref, +int *ref_flags,int need_symref);int repo_dwim_ref (struct repository *r,const char *str,int len, -struct object_id *oid,char **ref,int nonfatal_dangling_mark); +struct object_id *oid,char **ref,int nonfatal_dangling_mark, +int *ref_flags,int need_symref);int repo_dwim_log (struct repository *r,const char *str,int len,struct object_id *oid,char **ref);static inline int dwim_ref (const char *str,int len,struct object_id *oid,char **ref,int nonfatal_dangling_mark) {return repo_dwim_ref (the_repository, str, len, oid, ref, - nonfatal_dangling_mark); + nonfatal_dangling_mark,0 ,0 ); }int dwim_log (const char *str,int len,struct object_id *oid,char **ref);
upload-pack.c +1 -1
@@ -975,7 +975,7 @@ static int process_deepen_not(const char *line, struct string_list *deepen_not,if (skip_prefix (line,"deepen-not " , &arg)) {char *ref =NULL ;struct object_id oid; -if (expand_ref (the_repository, arg,strlen (arg), &oid, &ref) !=1 ) +if (expand_ref (the_repository, arg,strlen (arg), &oid, &ref,0 ,0 ) !=1 )die ("git upload-pack: ambiguous deepen-not: %s" , line);string_list_append (deepen_not, ref);free (ref);
Signature · 1 line
gitgitgadget
PATCH 2/4 ref-filter: provide ref_flags to pretty_print_ref()
In reply to ZheNing Hu via GitGitGadgetFrom: ZheNing Hu <[email protected]>
pretty_print_ref() creates a ref_item through new_ref_array_item(), but such ref_item does not set its flag member, this means that %(symref) will not be processed in populate_value().
So provide ref_flags parameter to pretty_print_ref() and verify_tag(). pretty_print_ref() has two callers: cmd_verify_tag(), verify_tag(), let for_each_tag_name() use read_ref_full() instread of read_ref() to generate the ref_flags and pass to pretty_print_ref(); However, cmd_verify_tag() needs to be modified later to support providing ref_flags to pretty_print_ref().
At the same time, let verify_tag() pass "ref" instread of "name" to gpg_verify_tag() and pretty_print_ref(), which can help pretty_print_ref get fullref name.
--- builtin/tag.c | 15 ++++++++------- builtin/verify-tag.c | 2 +- ref-filter.c | 3 ++- ref-filter.h | 2 +- 4 files changed, 12 insertions(+), 10 deletions(-)
builtin/tag.c +8 -7
@@ -83,7 +83,7 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, }typedef int (*each_tag_name_fn )(const char *name,const char *ref, -const struct object_id *oid,void *cb_data); +const struct object_id *oid,void *cb_data,int ref_flags);static int for_each_tag_name (const char **argv,each_tag_name_fn fn,void *cb_data)
@@ -92,16 +92,17 @@ static int for_each_tag_name(const char **argv, each_tag_name_fn fn,struct strbuf ref =STRBUF_INIT ;int had_error =0 ;struct object_id oid; +int ref_flags;for (p = argv; *p; p++) {strbuf_reset (&ref);strbuf_addf (&ref,"refs/tags/%s" , *p); -if (read_ref (ref.buf, &oid)) { +if (read_ref_full (ref.buf,RESOLVE_REF_READING , &oid, &ref_flags)) {error (_ ("tag '%s' not found." ), *p); had_error =1 ;continue ; } -if (fn (*p, ref.buf, &oid, cb_data)) +if (fn (*p, ref.buf, &oid, cb_data, ref_flags)) had_error =1 ; }strbuf_release (&ref);
@@ -109,7 +110,7 @@ static int for_each_tag_name(const char **argv, each_tag_name_fn fn, }static int collect_tags (const char *name,const char *ref, -const struct object_id *oid,void *cb_data) +const struct object_id *oid,void *cb_data,int unused_flags) {struct string_list *ref_list = cb_data;
@@ -143,7 +144,7 @@ static int delete_tags(const char **argv) }static int verify_tag (const char *name,const char *ref, -const struct object_id *oid,void *cb_data) +const struct object_id *oid,void *cb_data,int ref_flags) {int flags;struct ref_format *format = cb_data;
@@ -152,11 +153,11 @@ static int verify_tag(const char *name, const char *ref,if (format->format) flags =GPG_VERIFY_OMIT_STATUS ; -if (gpg_verify_tag (oid, name, flags)) +if (gpg_verify_tag (oid, ref, flags))return -1 ;if (format->format) -pretty_print_ref (name, oid, format); +pretty_print_ref (ref, oid, format, ref_flags);return 0 ; }
builtin/verify-tag.c +1 -1
@@ -71,7 +71,7 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) }if (format.format) -pretty_print_ref (name, &oid, &format); +pretty_print_ref (name, &oid, &format,0 ); }return had_error; }
ref-filter.c +2 -1
@@ -2632,7 +2632,7 @@ int format_ref_array_item(struct ref_array_item *info, }void pretty_print_ref (const char *name,const struct object_id *oid, -struct ref_format *format) +struct ref_format *format,int ref_flags) {struct ref_array_item *ref_item;struct strbuf output =STRBUF_INIT ;
@@ -2640,6 +2640,7 @@ void pretty_print_ref(const char *name, const struct object_id *oid, ref_item =new_ref_array_item (name, oid); ref_item->kind =ref_kind_from_refname (name); + ref_item->flag = ref_flags;if (format_ref_array_item (ref_item, format, &output, &err))die ("%s" , err.buf);fwrite (output.buf,1 , output.len, stdout);
ref-filter.h +1 -1
@@ -140,7 +140,7 @@ void setup_ref_filter_porcelain_msg(void);* name must be a fully qualified refname. */ void pretty_print_ref (const char *name,const struct object_id *oid, -struct ref_format *format); +struct ref_format *format,int ref_flags);/* * Push a single ref onto the array; this can be used to construct your own
Signature · 1 line
gitgitgadget
PATCH 3/4 verify_tag: use repo_dwim_ref() to get ref fullname and ref_flags
In reply to ZheNing Hu via GitGitGadgetFrom: ZheNing Hu <[email protected]>
Since verify-tag cannot currently use %(symref) correctly, we need to get the ref flags from pretty_print_ref() caller. So use repo_dwim_ref() in cmd_verify_tag() to get fullref name and ref flags and pass to pretty_print_ref(), which can help us fix %(symref) feature.
--- builtin/verify-tag.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
builtin/verify-tag.c +5 -1
@@ -59,6 +59,8 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)while (i < argc) {struct object_id oid;const char *name = argv[i++]; +char *refname; +int ref_flags;if (get_oid (name, &oid)) { had_error = !!error ("tag '%s' not found." , name);
@@ -70,8 +72,10 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)continue ; } +if (repo_dwim_ref (the_repository, name,strlen (name), &oid, &refname,0 , &ref_flags,1 )) + name = refname;if (format.format) -pretty_print_ref (name, &oid, &format,0 ); +pretty_print_ref (name, &oid, &format, ref_flags); }return had_error; }
Signature · 1 line
gitgitgadget
PATCH 4/4 ref-filter: let tag verify use %(refname:lstrip=2) by default
In reply to ZheNing Hu via GitGitGadgetFrom: ZheNing Hu <[email protected]>
Because git verify-tag or git tag --verify are using command line parameters we pass as the %(refname) output (e.g. git verify-tag --format="verify: %(refname)" c06b72d02 will output "verify: c06b72d02"). This is an design error, but it is widely used by some scripts.
After we fix the bug that git verify_tag and git tag --verify cannot use %(symref) correctly, we still need to make %(refname) keep the same output as in the past. So add special_tag_verify to ref_array_item and ref_format, when this flag is setted and we are using refname atom default output formats such as %(refname), %(symref), modify the attribute of the atom corresponding to %(refname) in get_refname() and get_symref() and fill_remote_ref_details(), which let show_ref() output something like %(refname:lstrip=2). After show_ref() completes, restore the atom attribute as their origin state.
We can set special_tag_verify flag directly in tag.c, but in verify_tag.c, if we didn't get a refname from repo_dwim_ref(), we don’t need to set special_tag_verify of ref_format and change its output form, otherwise, set it.
--- builtin/tag.c | 1 + builtin/verify-tag.c | 5 ++++- ref-filter.c | 44 +++++++++++++++++++++++++++++++++---------- ref-filter.h | 2 ++ t/t7030-verify-tag.sh | 23 ++++++++++++++++++++++ 5 files changed, 64 insertions(+), 11 deletions(-)
builtin/tag.c +1 -0
@@ -149,6 +149,7 @@ static int verify_tag(const char *name, const char *ref,int flags;struct ref_format *format = cb_data; flags =GPG_VERIFY_VERBOSE ; + format->special_tag_verify =1 ;if (format->format) flags =GPG_VERIFY_OMIT_STATUS ;
builtin/verify-tag.c +4 -1
@@ -61,6 +61,7 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)const char *name = argv[i++];char *refname;int ref_flags; + format.special_tag_verify =0 ;if (get_oid (name, &oid)) { had_error = !!error ("tag '%s' not found." , name);
@@ -72,8 +73,10 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)continue ; } -if (repo_dwim_ref (the_repository, name,strlen (name), &oid, &refname,0 , &ref_flags,1 )) +if (repo_dwim_ref (the_repository, name,strlen (name), &oid, &refname,0 , &ref_flags,1 )) { name = refname; + format.special_tag_verify =1 ; + }if (format.format)pretty_print_ref (name, &oid, &format, ref_flags); }
ref-filter.c +34 -10
83 lines not on this page · raw message
ref-filter.h +2 -0
@@ -42,6 +42,7 @@ struct ref_array_item {int flag;unsigned int kind;const char *symref; +int special_tag_verify;struct commit *commit;struct atom_value *value;char refname[FLEX_ARRAY ];
@@ -84,6 +85,7 @@ struct ref_format {/* Internal state to ref-filter */ int need_color_reset_at_eol; +int special_tag_verify; };#define REF_FORMAT_INIT { .use_color = -1 }
t/t7030-verify-tag.sh +23 -0
@@ -194,6 +194,29 @@ test_expect_success GPG 'verifying tag with --format' ' test_cmp expect actual ' +test_expect_success GPG 'verifying tag with --format="%(refname) %(symref)"' ' + git tag -s -m bar annotated && + git symbolic-ref refs/tags/symref refs/tags/annotated && + sha=$(git rev-parse symref) && + SP=" " && + cat >expect <<-EOF && + verify: annotated$SP + verify: symref annotated + verify: $sha$SP + EOF + git verify-tag --format="verify: %(refname) %(symref)" annotated symref $sha >actual && + test_cmp expect actual +' + +test_expect_success GPG 'tag verify with --format="%(refname) %(symref)"' ' + cat >expect <<-EOF && + verify: annotated$SP + verify: symref annotated + EOF + git tag --verify --format="verify: %(refname) %(symref)" annotated symref >actual && + test_cmp expect actual +' + test_expect_success GPG 'verifying tag with --format="%(rest)" must fail' ' test_must_fail git verify-tag --format="%(rest)" "fourth-signed" '
Signature · 1 line
gitgitgadget