I will be taking over a series that Josh was working on before he went on parental leave. For this RFC, I was wondering whether this approach to blocking users from eventually commiting invalid submodules is feasible. Eg. should we block at commit time instead? And if so, how would that approach look? === Cover letter === At $dayjob, we believe that we need to change the default behavior around adding embedded repositories. Currently, we issue advice that embedded repositories cannot be checked out when the top-level repo is cloned, and that the user should probably use a submodule instead. However, our experience is that many users ignore this advice, and commit embedded repositories to shared projects. This causes toil for repo administrators who must correct these mistakes. The main point of this series is to change the warning issued in this case into a fatal error. However, this breaks lots of test cases that were implemented prior to the original creation of the warning, so the bulk of the changes are test cleanups. For reviewer convenience, I've tried to group the test changes with similar structure into the same commits: * Patch 1: Fixes a leak in submodule-config.c * Patch 2: Move setup code into test_expect blocks * Patch 3: Substitute `git submodule add` instead of `git add` for simple cases (no other adjustments necessary) * Patch 4: As in patch 2, but also adjust expected diff output * Patch 5: As in patch 2, but also adjust expected status output * Patch 6: Change the embedded repo warning to a fatal error An open question: * Currently we can bypass the embedded repo check with `--no-warn-embedded-repo`. I've kept the name of the flag the same for backwards compatibility. Should we instead rename the flag? Calvin Wan (1): leak fix: cache_put_path Josh Steadmon (5): t4041, t4060: modernize test style tests: Use `git submodule add` instead of `git add` tests: use `git submodule add` and fix expected diffs tests: use `git submodule add` and fix expected status add: reject nested repositories Documentation/git-add.txt | 7 +- builtin/add.c | 28 ++- submodule-config.c | 4 +- t/t0008-ignores.sh | 2 +- t/t2013-checkout-submodule.sh | 4 +- t/t2103-update-index-ignore-missing.sh | 2 +- t/t2107-update-index-basic.sh | 2 +- t/t3040-subprojects-basic.sh | 5 +- t/t3050-subprojects-fetch.sh | 3 +- t/t3404-rebase-interactive.sh | 3 +- t/t3701-add-interactive.sh | 5 +- t/t4010-diff-pathspec.sh | 2 +- t/t4020-diff-external.sh | 2 +- t/t4027-diff-submodule.sh | 17 +- t/t4035-diff-quiet.sh | 1 + t/t4041-diff-submodule-option.sh | 228 +++++++++++++++---- t/t4060-diff-submodule-option-diff-format.sh | 224 +++++++++++++----- t/t5531-deep-submodule-push.sh | 4 +- t/t6416-recursive-corner-cases.sh | 12 +- t/t6430-merge-recursive.sh | 1 + t/t6437-submodule-merge.sh | 12 +- t/t7400-submodule-basic.sh | 4 +- t/t7401-submodule-summary.sh | 4 +- t/t7402-submodule-rebase.sh | 2 +- t/t7412-submodule-absorbgitdirs.sh | 2 +- t/t7414-submodule-mistakes.sh | 21 +- t/t7450-bad-git-dotfiles.sh | 2 +- t/t7506-status-submodule.sh | 15 +- t/t7508-status.sh | 134 +++++++++-- 29 files changed, 569 insertions(+), 183 deletions(-) -- 2.39.1.581.gbfd45094c4-goog
