Re: [PATCH 3/3] fast-export: output reset command for commandline revs

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 7, 2011, 05:52 UTC
Message-ID: <7v1utk4gym.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1320535407-4933-4-git-send-email-srabbelier@gmail.com>

Sverre Rabbelier <srabbelier@gmail.com> writes:

>   Additionally, the elem->whence != REV_CMD_RIGHT case should really
>   check if REV_CMD_RIGHT_REF, but as this is not provided by the
>   ref_info structure this is left as is.

I am not sure what you mean by REV_CMD_RIGHT_REF here. Do you mean "We are only interested in the RHS endpoint of A...B syntax (i.e. B) but only when it is a refname and not an arbitrary SHA-1 expression (e.g. even though next~4 in "master...next~4" is a RHS endpoint, it is not a ref, and we do not want it)"?

I think the distinction you are trying to express ("is it a ref and if so what exact refname resolve_ref() would produce, or is it just the name of a random commit?") is a very useful thing in general, but it is orthogonal to what existing REV_CMD_* are trying to express, which is "where did they come from", that you can read from the name of the field "whence".

Perhaps we would want to add a new field "const char *ref" to "struct rev_cmdline_entry" to record the additional information you want perhaps by storing the result of resolve_ref() if it is a ref and NULL otherwise. Would it be too much work to add it to perfect this series?

By the way, REV_CMD_REF is meant to mean "the user did not explicitly name this but it came as a result of iterating over refs/something/ namespace", and does not mean "this is a tip of some ref" (they happen to be all refs, but "obtained by iteration, not by explicit naming" is the more important reason for marking them as such). As they are numerous, if you are going to add that "const char *ref" field to rev_cmdline_entry, we may want to either leave it NULL for REV_CMD_REF entries (the name field already has that information anyway), or have it point at its name field (we need to audit the codepath to free the name and ref fields if we go that route).