Re: [PATCH 5/6] log: pass rev_info to git_log_config()

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 4, 2012, 07:05 UTC
Message-ID: <7v1uhe3efa.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1349314419-8397-6-git-send-email-gitster@pobox.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 14 quoted lines

> Call init_revisions() first to prepare the revision traversal
> parameters and pass it to git_log_config(), so that necessary bits
> in the traversal parameters can be tweaked before we call the
> command line parsing infrastructure setup_revisions() from
> the cmd_log_init_finish() function.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>
>  * This is made separate from the next one that touches the contents
>    of "rev" to make sure the existing code does not depend on the
>    current initialization order.  I do not think it does but better
>    be careful to keep the history easier to bisect, than be sorry
>    when an issue does appear.

And I was right X-<. This does break the assumption the recent diff.context series makes.

What happens is that

    - init_revisions() initializes revs->grep_filter; that is why this
      patch wanted to call it first, so that it can futz with it
      from git_config().

    - however, init_revisions() also calls diff_setup(), and the
      diff machinery initializes revs->diffopt->context from
      diff_context_default.  Compiled in default of this value is 3,
      but the diff.context series wants to update this variable with
      the configuration before this call happens.

So we would need to do something like:

    - call git_log_config() first to let diff_context_default
      updated from the configuration as before.  find the values of
      grep.* defaults at the same time, but stash it away in a
      separate "struct grep_opt" (yuck);

    - call init_revisions() and let it initialize revs->grep_filter
      and revs->diffopt as before;

    - copy the grep.* defaults we learned during git_log_config() to
      revs->grep_filter.

which is a bit yucky, but survivable.

I'll fix these two patches up later.