Re: "HEAD -> branch" decoration doesn't work with "--decorate=full"
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 13, 2015, 17:11 UTC
- Message-ID
- <xmqqwq0c9zc1.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <55534D95.60609@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
Show 13 quoted lines
> The new-style "HEAD -> branch" style decoration doesn't work when > "--decorate=full" is used: > >> $ bin-wrappers/git show --oneline --decorate >> c518059 (HEAD -> master, gitster/master) Merge branch 'maint' >> >> $ bin-wrappers/git show --oneline --decorate=full >> c518059 (HEAD, refs/remotes/gitster/master, refs/heads/master) Merge branch 'maint' > > I would have expected the second invocation to show "HEAD -> > refs/heads/master". > > Was that an oversight or a conscious decision?
I actually think this ultimately comes from a poor design of the name-decorations infrastructure. The program is expected to call load_ref_decorations() only once and make the choice between the full/short at that point, which is passed to add_ref_decoration() to record either 'refs/heads/master' or 'master' in the singleton name_decoration decoration. But it does not store which one was chosen by the caller of load_ref_decorations() anywhere in the subsystem.
When current_pointed_by_HEAD() wants to see if decorations on an object, e.g. 'master', matches what 'HEAD' resolves to, it cannot tell if the original set-up was done for the full decoration, and the current code just assumes (without even realizing that it is making that assumption) the decoration must have been set up for the short ones.
Perhaps something like this, but I am not committing it without tests or a proper log messge.
I moved "static loaded" outside as it is in the same category as the global name-decoration and decoration-flags, i.e. to be initialised once at the beginning to a fixed setting and then be used with that setting.
log-tree.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/log-tree.c b/log-tree.c index 2c1ed0f..92259bc 100644 --- a/log-tree.c +++ b/log-tree.c @@ -13,6 +13,8 @@ #include "line-log.h" static struct decoration name_decoration = { "object names" }; +static int decoration_loaded; +static int decoration_flags; static char decoration_colors[][COLOR_MAXLEN] = { GIT_COLOR_RESET, @@ -146,9 +148,9 @@ static int add_graft_decoration(const struct commit_graft *graft, void *cb_data) void load_ref_decorations(int flags) { - static int loaded; - if (!loaded) { - loaded = 1; + if (!decoration_loaded) { + decoration_loaded = 1; + decoration_flags = flags; for_each_ref(add_ref_decoration, &flags); head_ref(add_ref_decoration, &flags); for_each_commit_graft(add_graft_decoration, NULL); @@ -196,8 +198,19 @@ static const struct name_decoration *current_pointed_by_HEAD(const struct name_d branch_name = resolve_ref_unsafe("HEAD", 0, unused, &rru_flags); if (!(rru_flags & REF_ISSYMREF)) return NULL; - if (!skip_prefix(branch_name, "refs/heads/", &branch_name)) - return NULL; + + if ((decoration_flags == DECORATE_SHORT_REFS)) { + if (!skip_prefix(branch_name, "refs/heads/", &branch_name)) + return NULL; + } else { + /* + * Each decoration has a refname in full; keep + * branch_name also in full, but still make sure + * HEAD is a reasonable ref. + */ + if (!starts_with(branch_name, "refs/")) + return NULL; + } /* OK, do we have that ref in the list? */ for (list = decoration; list; list = list->next)