Re: git submodules implementation question

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 1, 2016, 18:32 UTC
Message-ID: <xmqqmvjrwjwm.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAN5XQftt3qVoU9gB2oyimY328VK0W6xq5FSCQYvcB9dEgkxVWA@mail.gmail.com>

Uma Srinivasan <usrinivasan@twitter.com> writes:

Show 5 quoted lines

> 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.

Show 9 quoted lines

>> --- 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");
>>  }
>>