Thread (2 messages) flat view 2 messages, 2 authors, 2016-06-15

Re: [PATCH v4 5/8] branch: drop non-commit error reporting

From: Karthik Nayak <hidden>
Date: 2016-06-15 23:06:34

On Tue, Sep 15, 2015 at 1:19 AM, Junio C Hamano [off-list ref] wrote:
Karthik Nayak [off-list ref] writes:
quoted
Remove the error reporting variable to make the code easier to port
over to using ref-filter APIs. This variable is not required as in
ref-filter we already check for possible errors and report them.
Hmmmm.  What do you exactly mean "possible errors" here?
The checking if it points to a commit.
Unlike generic refs that can point at anything, refs/heads/* refs
must point at a commit [*1*], and that is why the error message says
'does not point at a commit'.

Does ref-filter API have corresponding check to treat the local and
remote branch hierarchies differently from tags and others?
This check in the existing code is only done when one of these holds:
ref_list->verbose || ref_list->with_commit || merge_filter != NO_FILTER

There is a similar check in ref-filter when we filter and obtain refs.

    if (filter->merge_commit || filter->with_commit || filter->verbose) {
        commit = lookup_commit_reference_gently(oid->hash, 1);
        if (!commit)
            return 0;
        /* We perform the filtering for the '--contains' option */
        if (filter->with_commit &&
            !commit_contains(filter, commit))
            return 0;
    }
[Footnote]

*1* Strictly speaking, use of lookup_commit_reference_gently() in
    the existing code allows a committish to be there and does not
    limit the tip objects to be commits, but I think it is a bug.
    At least, even with deref_tag(), we reject non committish found
    at the tip of branch refs and explain with the error message
    this patch removes.
Ah, didn't go through this.

-- 
Regards,
Karthik Nayak
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help