rev-parse vs. rev-list --no-walk

rev-parse vs. rev-list --no-walk

[Michael J Gruber]

In connection with an upcoming RFD patch, I noticed that rev-parse and
rev-list --no-walk do very similar things (OK - I knew *that* already)
in different ways. In particular, rev-list uses the ordinary
setup_revision() which is used in many other places, whereas rev-parse
does it's own thing.

rev-parse does a couple more things, of course, but why doesn't it use
setup_revision() ? I just wanted to ask before trying to restructure
things....

As it is we have the maintenance burden of keeping both rev option
parsers in sync. And, in fact, they are not:

- setup_revision() (and thus rev-list) explicitly adds HEAD for "--all",
rev-parse does not

- setup_revision() outputs each sha1 only once, rev-parse possibly
multiple times (in case of coinciding heads/tags...)

Of course, rev-parse and rev-list --no-walk do actually very different
things, as one can see from the results of "--tags" on both (partially
answering my own question here). And that difference is intentional. So:

- Should setup_revision() really add HEAD on --all? (Probably yes.) In
this case I'd adjust the doc.

- Shouldn't rev-parse --all add HEAD as well?

- Is there any way to use setup_revision() without resolving tags to
commits etc. (and thus using it for rev-parse)?

Michael

Re: rev-parse vs. rev-list --no-walk

[Junio C Hamano]

Michael J Gruber <git@drmicha.warpmail.net> writes:

> rev-parse does a couple more things, of course, but why doesn't it use
> setup_revision() ? I just wanted to ask before trying to restructure
> things....

Because rev-parse was written as a helper for scripts that take revision
options and non-revision options, whose purpose was to sift the arguments
into four bins:

    - revision parameters       (e.g. HEAD, master~20)
    - revision flags		(e.g. --all, --parents)
    - non-revision parameters	(e.g. Makefile, hello.c)
    - non-revision flags	(e.g. --stat, -p)

as its primary customer was things like whatchanged that used to be
scripted this way:

	git rev-list rev-flags rev-params |
        git diff-tree --stdin non-rev-flags non-rev-params |
        less

If you call setup_revisions(), it will call handle_revision_opt() which in
turn will call diff_opt_parse(), so I suspect you wouldn't be able to sift
between rev-flags and non-rev-flags.

I think one sane thing to do is to stop adding new rev-flags revision.c
supports to rev-parse (it already lags behind and nobody complained that
rev-parse doesn't understand --first-parent as a rev-flag), and keep its
use as "revision and non revision option sifter" only to support older
scripts written back in v1.0.0 days.  Its primary use these days is "turn
symbolic object names into 40-letter SHA-1", but "option sifter" aspect of
the command seems to have outlived its usefulness.

Re: rev-parse vs. rev-list --no-walk

[Jay Soffian]

On Sat, May 1, 2010 at 4:05 PM, Junio C Hamano <gitster@pobox.com> wrote:
> I think one sane thing to do is to stop adding new rev-flags revision.c
> supports to rev-parse (it already lags behind and nobody complained that
> rev-parse doesn't understand --first-parent as a rev-flag), and keep its
> use as "revision and non revision option sifter" only to support older
> scripts written back in v1.0.0 days.  Its primary use these days is "turn
> symbolic object names into 40-letter SHA-1", but "option sifter" aspect of
> the command seems to have outlived its usefulness.

We tell scripters to be careful to use the plumbing and not the
porcelain. From that standpoint, shouldn't we do our best to prevent
the plumbing from falling into disrepair?

(Or perhaps I'm reading too much into what you say.)

j.

Re: rev-parse vs. rev-list --no-walk

[Michael J Gruber]

Jay Soffian venit, vidit, dixit 03.05.2010 17:22:
> On Sat, May 1, 2010 at 4:05 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> I think one sane thing to do is to stop adding new rev-flags revision.c
>> supports to rev-parse (it already lags behind and nobody complained that
>> rev-parse doesn't understand --first-parent as a rev-flag), and keep its
>> use as "revision and non revision option sifter" only to support older
>> scripts written back in v1.0.0 days.  Its primary use these days is "turn
>> symbolic object names into 40-letter SHA-1", but "option sifter" aspect of
>> the command seems to have outlived its usefulness.
> 
> We tell scripters to be careful to use the plumbing and not the
> porcelain. From that standpoint, shouldn't we do our best to prevent
> the plumbing from falling into disrepair?

According to git(1), git rev-list is low-level, but git rev-parse is not!

>From a cursory look at current source it would be simple to discourage
use of rev-parse for revision flags (and, probably, as an option
sifter). There's no need to remove them (and break some outside
scripts). Junio suggested not to add any new ones to rev-parse. That
would make my RFD patch (--heads, --locals) smaller, and answers the
questions about solving the difference between rev-parse and rev-list
"-all" regarding HEAD. But maybe git-rev-parse(1) should change then, in
order not to encourage its use as option sifter. Everyone stops by this
man page when looking up revision syntax.

Michael