Subject: Re: [PATCH 12/17] update submodules: add submodule_move_head

From: Brandon Williams <[email-masked]>
Date: Mon, 13 Mar 2017 10:24:21 -0700

On 03/11, Junio C Hamano wrote:
> Brandon Williams <[email-masked]> writes:
> 
> >> diff --git a/submodule.c b/submodule.c
> >> index 0b2596e88a..bc5fecf8c5 100644
> >> --- a/submodule.c
> >> +++ b/submodule.c
> >> @@ -1239,6 +1239,141 @@ int bad_to_remove_submodule(const char *path, unsigned flags)
> >>  	return ret;
> >>  }
> >>  
> >> +static int submodule_has_dirty_index(const struct submodule *sub)
> >> +{
> >> +	struct child_process cp = CHILD_PROCESS_INIT;
> >> +
> >> +	prepare_submodule_repo_env_no_git_dir(&cp.env_array);
> >> +
> >> +	cp.git_cmd = 1;
> >> +	argv_array_pushl(&cp.args, "diff-index", "--quiet", \
> >> +					"--cached", "HEAD", NULL);
> >
> > The formatting of this line is a little odd.  Also you can drop the
> > backslash.
> 
> Heh.  I think I saw and pointed out the same during the review of
> the previous round X-<.  It is a bit disappointing.
> 
> >> +static void submodule_reset_index(const char *path)
> >> +{
> >> +	struct child_process cp = CHILD_PROCESS_INIT;
> >> +	prepare_submodule_repo_env_no_git_dir(&cp.env_array);
> >> +
> >> +	cp.git_cmd = 1;
> >> +	cp.no_stdin = 1;
> >> +	cp.dir = path;
> >> +
> >> +	argv_array_pushf(&cp.args, "--super-prefix=%s/", path);
> >> +	argv_array_pushl(&cp.args, "read-tree", "-u", "--reset", NULL);
> >> +
> >> +	argv_array_push(&cp.args, EMPTY_TREE_SHA1_HEX);
> 
> Somewhat related; will this use of --super-prefix be affected when
> we split it into two for "adjust pathspec" prefix and "adjust
> output" prefix?

Maybe? From what we discussed we can have 'super_prefix' maintain its
current meaning (Path from root of parent down to the submodule) while
adding a new env var or option to pass the 'prefix' variable to the
child process (Path from root of parent to the directory the original
cmd was executed in).

So this shouldn't be affected unless we decide to change the semantics
of 'super_prefix'.

And I'm always torn on how best to do naming for the newer
functionality.  I think the easiest thing to do would be to have a
--prefix option to pass the 'prefix' from one process to the next, but
I'm sure there are other opinions on the matter.

-- 
Brandon Williams