Uma Srinivasan <usriniva...@twitter.com> writes:

> Yes, this one line fix addresses my problem.
>
> So, what is the next step? Will someone submit a patch or should I?
> Please note that I've never submitted a patch before, but I don't mind
> learning how to.

The final version needs to be accompanied with tests to show the
effect of this change for callers.  A test would set up a top-level
and submodule, deliberately break submodule/.git/ repository and
show what breaks and how without this change.

For example, there is a call in ok_to_remove_submodule() to run "git
status" in the submodule directory.  A test writer needs to find who
calls ok_to_remove_submodule() in what codepath (e.g. builtin/rm.c).
Then after figuring out under what condition the check is triggered,
write a test that runs "git rm" in a way that this change makes a
difference.  Hopefully, the test will fail without this change, and
will pass with this change.

So it is a bit more involved than just a single liner patch with one
paragraph log message.  I may be able to find or make some time
after I tag the 2.10 final this weekend to do so myself.

>> --- a/submodule.c
>> +++ b/submodule.c
>> @@ -1160,4 +1160,5 @@ void prepare_submodule_repo_env(struct argv_array *out)
>>                 if (strcmp(*var, CONFIG_DATA_ENVIRONMENT))
>>                         argv_array_push(out, *var);
>>         }
>> +       argv_array_push(out, "GIT_DIR=.git");
>>  }
>>

Reply via email to