Re: [RFC PATCH v2 12/12] submodule: remove the .gitmodules file when it is empty
From: Stefan Beller <hidden>
Date: 2018-08-02 21:16:08
On Thu, Aug 2, 2018 at 6:47 AM Antonio Ospite [off-list ref] wrote:
In particular this makes it possible to really clean things up when removing the last submodule with "git rm".
This sentence is a continuation of the subject line, and I had to reread it to follow along.
The rationale is that if git creates .gitmodules when adding the first submodule it should also remove it when removing the last submodule.
I agree with this sentiment. It seems slightly odd to me to have this tied in the same patch series that changes .gitmodules reading behavior as I could think of this feature as orthogonal to what this series achieved up to patch 10.
- test_cmp expect actual && + test_cmp expect.both_deleted actual &&
This seems to be the re-occuring pattern in t3600, and given that we have cat >expect <<EOF M .gitmodules D submod EOF cat >expect.both_deleted<<EOF D .gitmodules D submod EOF with no other writing of expect in the range, this seems to be correct. Maybe worth testing that we do not delete a .gitmodules file if we have more than one submodule? (But I would expect this to be covered implicitly somewhere in the test suite. If so that would be worth mentioning in the commit message instead of writing a test -- just looking quickly we do have " git rm --cached submodule2" in t7406 which might be sufficient?)
quoted hunk ↗ jump to hunk
test_expect_success "rm absorbs submodule's nested .git directory" 'diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index 48fd14fae6..2bb42a4c8f 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh@@ -99,6 +99,17 @@ inspect() { ) } +test_expect_success 'removal of last submodule also removes empty .gitmodules' ' + ( + cd addtest && + test ! -d .git/modules &&
test_path_is_missing ?
+ git submodule add -q "$submodurl" first_submod && + test -e .gitmodules &&
test_path_is_file
+ git rm -f first_submod &&
Do we need to force it here?
+ test ! -e .gitmodules
test_path_is_missing Thanks for this series! Overall it was a pleasant read, though I had some comments. Thanks, Stefan