threads / discuss / 23657

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

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

## tl;dr

4 messages between May 1, 2010 and May 3, 2010.

replies: 3people: 3as markdown or json

Michael J Gruber· May 1, 2010, 18:35 UTC · lore

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
Junio C Hamano· May 1, 2010, 20:05 UTC · re: Michael J Gruber · lore

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

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.

Jay Soffian· May 3, 2010, 15:22 UTC · re: Junio C Hamano · lore

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

On Sat, May 1, 2010 at 4:05 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 7 quoted lines
> 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.
Michael J Gruber· May 3, 2010, 15:50 UTC · re: Jay Soffian · lore

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

Jay Soffian venit, vidit, dixit 03.05.2010 17:22:
Show 12 quoted lines
> 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

← back to recent threads