Re: git diff-index with relative git-dir does not work

[Yasushi SHOJI <yashi@atmark-techno.com>]

At Tue, 9 Feb 2010 18:58:51 +0700,
Nguyen Thai Ngoc Duy wrote:
> 
> On 2/9/10, Yasushi SHOJI <yashi@atmark-techno.com> wrote:
> >  ...
> >  This is because static variable 'base' in sha1_file_name is already
> >  assigned _before_ setup_work_tree() from cmd_diff_index() is
> >  called. setup_work_tree() eventually chdir to the given work tree dir,
> >  but we use the old base to generate object file path. And that cause
> >  open(2) to fail because the object file path and the current dir is
> >  not in sync any more.
> >
> >  So, is it correct to assume that we must call setup_work_tree()
> >  _before_ any function which call getter/setter in environment.c?  This
> >  including open_sha1_file, in this case.
> 
> We must if gitdir is relative to cwd (and will be moved by
> setup_work_tree). Or just make gitdir absolute path.

ok. thanks.

> >  Also, would it be a good idea to make all builtin command to
> >  _explicitly_ call setup_* functions, so that we can find calling order
> >  bug?
> 
> If you agree that writing "RUN_SETUP" in git.c is explicit, then all
> builtin commands do explictly call setup_*. It's about relative
> directories and cwd being moved around.

oops.  I had omitted too much words.

In the diff-index case, it, indeed, has RUN_SETUP explicitly
set. however, it does not have NEED_WORK_TREE set.  And, this is
correct in the current semantics because diff-index is a tool to
compare the index and the object store. it does not need a work tree.

However, diff-index is used in describe which need a work tree if
--dirty is given.  That means that diff-index might be called
with --work-tree.

> >  In that case, we must change the setup functions signature to
> >  allow marking "not interested" or something.
> 
> I'm not sure I get your idea.

Given that in the current form of git, many built-in command is called
by many other built-in commands. It is hard to predict what is needed
and what's not.  Plus, --git-dir and --work-tree are options to git
itself not built-in's.  So, I thought it might be a good idea to call,
say, setup_work_tree_with_abs_path(), regardless of NEED_WORK_TREE, to
explicitly setup run time environment before any other part of the
code call, say, open_sha1_file.

If calling those functions are not acceptable due to speed or other
issues, making them debug / poison code might be enough.
-- 
         yashi