Re: [RFC/PATCHv2 5/5] grep: add support for grepping in submodules

[Nguyen Thai Ngoc Duy <pclouds@gmail.com>]

On Mon, Oct 18, 2010 at 9:01 AM, Chris Packham <judge.packham@gmail.com> wrote:
> On 17/10/10 03:28, Nguyen Thai Ngoc Duy wrote:
>> On Sat, Oct 16, 2010 at 6:26 AM, Chris Packham <judge.packham@gmail.com> wrote:
>>> +static int grep_submodule(struct grep_opt *opt, const char *path,
>>> +                         const char *sha1, const char *tree_name)
>>> +{
>>> +       struct strbuf buf = STRBUF_INIT;
>>> +       struct strbuf pre_buf = STRBUF_INIT;
>>> +       struct child_process cp;
>>> +       const char **argv = create_sub_grep_argv(opt, path, sha1, tree_name);
>>
>> Can we just save sha1 in a env variable and drop this argv rewrite?
>>
> The tree_name is a hangover from when I was passing it as a command line
> arg, I'll remove it. For now we still need to rewrite argv to modify the
> pathspec and max-depth, eventually I'd like to ditch that approach in
> favour of using fork() and modifying opts.

You would also need to undo git repo setup, i.e. resetting static
variables in environment.c and setup.c, maybe some more. Also fork()
is not available on Windows.
Spawning new processes is the only feasible way I can think of now (or
even in the next year).

>>> +       const char *git_dir;
>>> +       int hit = 0;
>>> +       memset(&cp, 0, sizeof(cp));
>>> +
>>> +       strbuf_addf(&buf, "%s/.git", path);
>>> +       git_dir = read_gitfile_gently(buf.buf);
>>> +       if (!git_dir)
>>> +               git_dir = buf.buf;
>>> +       if (!is_directory(git_dir))
>>> +               goto out_free;
>>> +
>>> +       setenv("GIT_SUPER_REFNAME", tree_name, 1);
>>> +       setenv(GIT_DIR_ENVIRONMENT, git_dir, 1);
>>> +       setenv(GIT_WORK_TREE_ENVIRONMENT, path, 1);
>>
>> cp.env can be used to set these variables
>>
> I was struggling a little with how to do this. The run_command
> invocation in submodules.c uses local_repo_env which is protected
> against modification. I also don't understand how the actual values are
> passed. If someone could point me in the right direction it'd be
> appreciated.

From what I read in run-command.c,the current env var set is copied
as-is for new process, then any new variables from cp.env will be
added. Old variables can also be unset by saying "VARIABLE=" in
cp.env. The final env var set will be used for new process. Current
env vars are untouched. Something like this should work:

const char *env[4];

env[0] = malloc_sprintf("GIT_SUPER_REFNAME=%s", tree_name);
env[1] = malloc_sprintf("GIT_DIR_ENVIRONMENT=%s", git_dir);
env[2] = malloc_sprintf("GIT_WORK_TREE+ENVIRONMENT=%s", path);
env[3] = NULL;
cp.env = env;

>>> +       cp.argv = argv;
>>> +       cp.git_cmd = 1;
>>> +       cp.no_stdin = 1;
>>
>> I think you stll need "cp.dir = path;" here because the setup routines
>> won't do that for you. But I need to check/test that.
>>
> Originally I did set cp.dir but that is used by run_command (via
> start_command) to chdir. Which foils the cwd to worktree detection (at
> least in V1 of that patch).

Right. I forgot that. But then when new grep is run, it should move
back to worktree. I believe current grep code assumes that cwd is
worktree root.
-- 
Duy