At Tue, 9 Feb 2010 18:58:51 +0700,
Nguyen Thai Ngoc Duy wrote:
>
> On 2/9/10, Yasushi SHOJI <yashi@xxxxxxxxxxxxxxxxx> 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
--
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