2010/6/28 Jeff King <peff@xxxxxxxx>:
> On Mon, Jun 28, 2010 at 05:13:35AM -0500, Jonathan Nieder wrote:
>
>> > But reading the message for patch 4/4, I can't help but wonder if the
>> > right way forward is for the git wrapper to _always_ find the repository
>> > as the very first thing.
>> [...]
>> > the worst case should be a little bit of
>> > wasted effort.
>>
>> This is a very useful thought. Git without the chdir() to toplevel
>> would be pleasanter, I think. (Yes, I realize you weren’t necessarily
>> suggesting suppressing a chdir_to_toplevel() in place of
>> setup_git_repository() and friends.)
>
> I had just meant that we would not do the chdir() initially, but would
> do so right before running the actual command which wanted repository
> setup (and commands like init which do not do that setup would never
> chdir to the toplevel). But we must always run at least aliases and
> external sub-commands from the toplevel to keep backwards compatibility.
I think I'm missing something here. Looking up aliases means
repository search must be done in git wrapper anyway. Even if we don't
have to do repo search in git wrapper, the very first thing a sub
command does is likely git_config(), which will need repo again. As
long as repo search does not have any side effects, everything should
be fine, IMO.
> So yes, we are adding the extra lookup for commands like "git clone",
> but I suspect in practice nobody will care. If it is a big deal, we can
> do something like:
>
> if (!strcmp(cmd, "clone") || !strcmp(cmd, "init"))
> ... don't do setup ...
>
If git wrapper does not need setup (i.e. aliases) at all, that should
happen without hacks. "clone" and "init" do not have RUN_SETUP (nor
RUN_SETUP_GENTLY in my series), so there should not be any setup
before cmd_{init,clone} is run.
PS. I haven't forgotten my cleanup series. Day job has been taking too
much of my energy unfortunately.
--
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html