Re: [PATCH 0/4] git --paginate: do not commit pager choice too early

On Mon, Jun 28, 2010 at 05:13:35AM -0500, Jonathan Nieder wrote:

quoted

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.

[...]

quoted

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.

Regarding the repository search: automounters can cause pain if
DISCOVERY_ACROSS_FILESYSTEM is set and GIT_CEILING_DIRECTORIES is not
set appropriately.  An important script that does

  tmpdir=$(mktemp -d)
  cd "$tmpdir"
  ...

  git diff --no-index a b
  ...

could hang indefinitely if some nut has created a named pipe at
/tmp/.git.  I haven’t come up with any non far-fetched reason to mind
the repository search, though.

Will think about this.

Yeah, I considered that, too. But I doubt it is a big problem. If it
were, then just running "git log" when you are not in a repository would
be similarly painful. The people who have setups like that are already
having to deal with GIT_CEILING_DIRECTORIES to make things less painful
(though I imagine the new cross-filesystem discovery code will prevent
most of the pain without them having to actually do anything).

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 ...

which I admit is a terrible hack, but it is an optimization, not a
correctness thing. So we are not shutting out external user-defined
commands that might not care about the repo. We are just making our
common built-in ones a bit more efficient.

-Peff