From: Calvin Wan <calvinwan@google.com>
To: git@vger.kernel.org
Cc: Calvin Wan <calvinwan@google.com>,
steadmon@google.com, peff@peff.net, gitster@pobox.com
Subject: [PATCH v2 0/6] add: block invalid submodules
Date: Tue, 28 Feb 2023 18:52:53 +0000 [thread overview]
Message-ID: <20230228185253.2356546-1-calvinwan@google.com> (raw)
In-Reply-To: <20230213182134.2173280-1-calvinwan@google.com>
A couple of significant changes were made since the RFC. From the
feedback gathered it seems like the idea of this series is worth
pursuing -- thanks Peff and Junio for the reviews!
In the initial RFC, we continued using `--no-warn-embedded-repo` to
allow the user to add an embedded repo, but the naming of that option
was not ideal. This reroll adds `--allow-embedded-repo` and deprecates
`--no-warn-embedded-repo`.
While I initially claimed that there were incorrectly setup .gitmodules
tests with duplicate paths, it turns out that was caused by patch 4/6 of
this series. I added a new patch (see 5/6) that fixes the issue.
Subsequently, this means that the memory leak does not occur any more
(maybe I should still include that patch -- it's an edge case that comes
from a manually misconfigured .gitmodules).
Lastly I cleaned up the advice message and some of the tests --
replacing `git add` with `git submodule add` only in submodule specific
cases and some style cleanups.
Calvin Wan (1):
tests: remove duplicate .gitmodules 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 | 15 +-
builtin/add.c | 37 ++-
builtin/submodule--helper.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 | 58 ++---
t/t4035-diff-quiet.sh | 2 +-
t/t4041-diff-submodule-option.sh | 232 +++++++++++++----
t/t4060-diff-submodule-option-diff-format.sh | 226 ++++++++++++-----
t/t5531-deep-submodule-push.sh | 4 +-
t/t6416-recursive-corner-cases.sh | 12 +-
t/t6430-merge-recursive.sh | 2 +-
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 | 29 ++-
t/t7450-bad-git-dotfiles.sh | 2 +-
t/t7506-status-submodule.sh | 15 +-
t/t7508-status.sh | 248 ++++++++++++-------
29 files changed, 649 insertions(+), 291 deletions(-)
Range-diff against v1:
1: 34eacaabbd < -: ---------- leak fix: cache_put_path
2: 2da941fc59 ! 1: 3efc25bf0e t4041, t4060: modernize test style
@@ Commit message
easier to diagnose, because errors will caught immediately rather than
in later unrelated test_expect blocks.
+ While at it, have tests clean up after themselves with
+ test_when_finished and fix other minor style issues.
+
Signed-off-by: Josh Steadmon <steadmon@google.com>
Signed-off-by: Calvin Wan <calvinwan@google.com>
@@ t/t4041-diff-submodule-option.sh: commit_file () {
-fullhead1=$(cd sm1; git rev-parse --verify HEAD)
+test_expect_success 'setup' '
+ test_create_repo sm1 &&
-+ add_file . foo >/dev/null &&
++ add_file . foo &&
+ head1=$(add_file sm1 foo1 foo2) &&
-+ fullhead1=$(cd sm1 && git rev-parse --verify HEAD)
++ fullhead1=$(git -C sm1 rev-parse --verify HEAD)
+'
test_expect_success 'added submodule' '
@@ t/t4041-diff-submodule-option.sh: test_expect_success 'modified submodule(forwar
-fullhead2=$(cd sm1; git rev-parse --verify HEAD)
test_expect_success 'modified submodule(forward) --submodule=short' '
-+ fullhead2=$(cd sm1 && git rev-parse --verify HEAD) &&
++ fullhead2=$(git -C sm1 rev-parse --verify HEAD) &&
git diff --submodule=short >actual &&
cat >expected <<-EOF &&
diff --git a/sm1 b/sm1
@@ t/t4041-diff-submodule-option.sh: test_expect_success 'modified submodule(backwa
test_expect_success 'typechanged submodule(submodule->blob), --cached' '
git diff --submodule=log --cached >actual &&
+@@ t/t4041-diff-submodule-option.sh: test_expect_success 'typechanged submodule(submodule->blob), --cached' '
+ '
+
+ test_expect_success 'typechanged submodule(submodule->blob)' '
++ test_when_finished rm -rf sm1 &&
+ git diff --submodule=log >actual &&
+ cat >expected <<-EOF &&
+ diff --git a/sm1 b/sm1
@@ t/t4041-diff-submodule-option.sh: test_expect_success 'typechanged submodule(submodule->blob)' '
test_cmp expected actual
'
@@ t/t4041-diff-submodule-option.sh: test_expect_success 'typechanged submodule(sub
-rm -rf sm1 &&
-git checkout-index sm1
test_expect_success 'typechanged submodule(submodule->blob)' '
-+ rm -rf sm1 &&
++ test_when_finished rm -f sm1 &&
+ git checkout-index sm1 &&
git diff-index -p --submodule=log HEAD >actual &&
cat >expected <<-EOF &&
@@ t/t4041-diff-submodule-option.sh: test_expect_success 'typechanged submodule(sub
-head6=$(add_file sm1 foo6 foo7)
-fullhead6=$(cd sm1; git rev-parse --verify HEAD)
test_expect_success 'nonexistent commit' '
-+ rm -f sm1 &&
+ test_create_repo sm1 &&
+ head6=$(add_file sm1 foo6 foo7) &&
-+ fullhead6=$(cd sm1 && git rev-parse --verify HEAD) &&
++ fullhead6=$(git -C sm1 rev-parse --verify HEAD) &&
git diff-index -p --submodule=log HEAD >actual &&
cat >expected <<-EOF &&
Submodule sm1 $head4...$head6 (commits not present)
@@ t/t4041-diff-submodule-option.sh: test_expect_success 'typechanged submodule(blo
git diff-index -p --submodule=log HEAD >actual &&
test_must_be_empty actual
'
+@@ t/t4041-diff-submodule-option.sh: test_expect_success 'submodule contains untracked and modified content (dirty ig
+ '
+
+ test_expect_success 'submodule contains untracked and modified content (all ignored)' '
++ test_when_finished rm -f sm1/new-file &&
+ echo new > sm1/foo6 &&
+ git diff-index -p --ignore-submodules --submodule=log HEAD >actual &&
+ test_must_be_empty actual
+ '
+
+ test_expect_success 'submodule contains modified content' '
+- rm -f sm1/new-file &&
+ git diff-index -p --submodule=log HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 contains modified content
@@ t/t4041-diff-submodule-option.sh: test_expect_success 'submodule contains modified content' '
test_cmp expected actual
'
@@ t/t4041-diff-submodule-option.sh: test_expect_success 'submodule contains modifi
git diff-index -p --submodule=log HEAD >actual &&
cat >expected <<-EOF &&
Submodule sm1 $head6..$head8:
+@@ t/t4041-diff-submodule-option.sh: test_expect_success 'modified submodule contains untracked and modified content
+ '
+
+ test_expect_success 'modified submodule contains untracked and modified content (all ignored)' '
++ test_when_finished rm -f sm1/new-file &&
+ echo modification >> sm1/foo6 &&
+ git diff-index -p --ignore-submodules --submodule=log HEAD >actual &&
+ test_must_be_empty actual
+ '
+
+ test_expect_success 'modified submodule contains modified content' '
+- rm -f sm1/new-file &&
++ test_when_finished rm -rf sm1 &&
+ git diff-index -p --submodule=log HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 contains modified content
@@ t/t4041-diff-submodule-option.sh: test_expect_success 'modified submodule contains modified content' '
test_cmp expected actual
'
-rm -rf sm1
test_expect_success 'deleted submodule' '
-+ rm -rf sm1 &&
git diff-index -p --submodule=log HEAD >actual &&
cat >expected <<-EOF &&
- Submodule sm1 $head6...0000000 (submodule deleted)
@@ t/t4041-diff-submodule-option.sh: test_expect_success 'path filter' '
test_cmp expected actual
'
@@ t/t4041-diff-submodule-option.sh: test_expect_success 'given commit --submodule'
-fullhead7=$(cd sm2; git rev-parse --verify HEAD)
-
test_expect_success 'given commit --submodule=short' '
-+ fullhead7=$(cd sm2 && git rev-parse --verify HEAD) &&
++ fullhead7=$(git -C sm2 rev-parse --verify HEAD) &&
git diff-index -p --submodule=short HEAD^ >actual &&
cat >expected <<-EOF &&
diff --git a/sm1 b/sm1
@@ t/t4060-diff-submodule-option-diff-format.sh: test_expect_success 'modified subm
-fullhead2=$(cd sm1; git rev-parse --verify HEAD)
test_expect_success 'modified submodule(forward) --submodule=short' '
-+ fullhead2=$(cd sm1 && git rev-parse --verify HEAD) &&
++ fullhead2=$(git -C sm1 rev-parse --verify HEAD) &&
git diff --submodule=short >actual &&
cat >expected <<-EOF &&
diff --git a/sm1 b/sm1
@@ t/t4060-diff-submodule-option-diff-format.sh: test_expect_success 'modified subm
test_expect_success 'modified submodule(backward)' '
+ commit_file sm1 &&
+ head3=$(
-+ cd sm1 &&
-+ git reset --hard HEAD~2 >/dev/null &&
-+ git rev-parse --short --verify HEAD
++ git -C sm1 reset --hard HEAD~2 >/dev/null &&
++ git -C sm1 rev-parse --short --verify HEAD
+ ) &&
git diff-index -p --submodule=diff HEAD >actual &&
cat >expected <<-EOF &&
@@ t/t4060-diff-submodule-option-diff-format.sh: test_expect_success 'modified subm
test_expect_success 'typechanged submodule(submodule->blob), --cached' '
git diff --submodule=diff --cached >actual &&
+@@ t/t4060-diff-submodule-option-diff-format.sh: test_expect_success 'typechanged submodule(submodule->blob), --cached' '
+ '
+
+ test_expect_success 'typechanged submodule(submodule->blob)' '
++ test_when_finished rm -rf sm1 &&
+ git diff --submodule=diff >actual &&
+ cat >expected <<-EOF &&
+ diff --git a/sm1 b/sm1
@@ t/t4060-diff-submodule-option-diff-format.sh: test_expect_success 'typechanged submodule(submodule->blob)' '
diff_cmp expected actual
'
@@ t/t4060-diff-submodule-option-diff-format.sh: test_expect_success 'typechanged s
-rm -rf sm1 &&
-git checkout-index sm1
test_expect_success 'typechanged submodule(submodule->blob)' '
-+ rm -rf sm1 &&
++ test_when_finished rm -f sm1 &&
+ git checkout-index sm1 &&
git diff-index -p --submodule=diff HEAD >actual &&
cat >expected <<-EOF &&
@@ t/t4060-diff-submodule-option-diff-format.sh: test_expect_success 'typechanged s
-test_create_repo sm1 &&
-head6=$(add_file sm1 foo6 foo7)
test_expect_success 'nonexistent commit' '
-+ rm -f sm1 &&
+ test_create_repo sm1 &&
+ head6=$(add_file sm1 foo6 foo7) &&
git diff-index -p --submodule=diff HEAD >actual &&
@@ t/t4060-diff-submodule-option-diff-format.sh: test_expect_success 'typechanged s
head7=$(git -C sm1 rev-parse --short --verify HEAD) &&
git diff-index -p --submodule=diff HEAD >actual &&
test_must_be_empty actual
+@@ t/t4060-diff-submodule-option-diff-format.sh: test_expect_success 'submodule contains untracked and modified content (dirty ig
+ '
+
+ test_expect_success 'submodule contains untracked and modified content (all ignored)' '
++ test_when_finished rm -f sm1/new-file &&
+ echo new > sm1/foo6 &&
+ git diff-index -p --ignore-submodules --submodule=diff HEAD >actual &&
+ test_must_be_empty actual
+ '
+
+ test_expect_success 'submodule contains modified content' '
+- rm -f sm1/new-file &&
+ git diff-index -p --submodule=diff HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 contains modified content
@@ t/t4060-diff-submodule-option-diff-format.sh: test_expect_success 'submodule contains modified content' '
diff_cmp expected actual
'
@@ t/t4060-diff-submodule-option-diff-format.sh: test_expect_success 'submodule con
-(cd sm1; git commit -mchange foo6 >/dev/null) &&
-head8=$(cd sm1; git rev-parse --short --verify HEAD) &&
test_expect_success 'submodule is modified' '
-+ (cd sm1 && git commit -mchange foo6 >/dev/null) &&
-+ head8=$(cd sm1 && git rev-parse --short --verify HEAD) &&
++ (git -C sm1 commit -mchange foo6 >/dev/null) &&
++ head8=$(git -C sm1 rev-parse --short --verify HEAD) &&
git diff-index -p --submodule=diff HEAD >actual &&
cat >expected <<-EOF &&
Submodule sm1 $head7..$head8:
+@@ t/t4060-diff-submodule-option-diff-format.sh: test_expect_success 'modified submodule contains untracked and modified content
+ '
+
+ test_expect_success 'modified submodule contains untracked and modified content (all ignored)' '
++ test_when_finished rm -f sm1/new-file &&
+ echo modification >> sm1/foo6 &&
+ git diff-index -p --ignore-submodules --submodule=diff HEAD >actual &&
+ test_must_be_empty actual
+@@ t/t4060-diff-submodule-option-diff-format.sh: test_expect_success 'modified submodule contains untracked and modified content
+
+ # NOT OK
+ test_expect_success 'modified submodule contains modified content' '
+- rm -f sm1/new-file &&
++ test_when_finished rm -rf sm1 &&
+ git diff-index -p --submodule=diff HEAD >actual &&
+ cat >expected <<-EOF &&
+ Submodule sm1 contains modified content
@@ t/t4060-diff-submodule-option-diff-format.sh: test_expect_success 'modified submodule contains modified content' '
diff_cmp expected actual
'
-rm -rf sm1
test_expect_success 'deleted submodule' '
-+ rm -rf sm1 &&
git diff-index -p --submodule=diff HEAD >actual &&
cat >expected <<-EOF &&
- Submodule sm1 $head7...0000000 (submodule deleted)
@@ t/t4060-diff-submodule-option-diff-format.sh: test_expect_success 'path filter' '
diff_cmp expected actual
'
@@ t/t4060-diff-submodule-option-diff-format.sh: test_expect_success 'deleted submo
-echo submodule-to-blob>sm2
-
test_expect_success 'typechanged(submodule->blob) submodule with .git file' '
++ test_when_finished "rm sm2 && mv sm2-bak sm2" &&
+ echo submodule-to-blob>sm2 &&
git diff-index -p --submodule=diff HEAD >actual &&
cat >expected <<-EOF &&
@@ t/t4060-diff-submodule-option-diff-format.sh: test_expect_success 'typechanged(s
-mv sm2-bak sm2
-
test_expect_success 'setup nested submodule' '
-+ rm sm2 &&
-+ mv sm2-bak sm2 &&
git -c protocol.file.allow=always -C sm2 submodule add ../sm2 nested &&
git -C sm2 commit -a -m "nested sub" &&
- head10=$(git -C sm2 rev-parse --short --verify HEAD)
@@ t/t4060-diff-submodule-option-diff-format.sh: test_expect_success 'diff --submodule=diff recurses into nested submodules' '
diff_cmp expected actual
'
@@ t/t4060-diff-submodule-option-diff-format.sh: test_expect_success 'diff --submod
test_expect_success 'diff --submodule=diff recurses into deleted nested submodules' '
+ (cd sm2 && commit_file nested) &&
+ commit_file sm2 &&
-+ head12=$(cd sm2 && git rev-parse --short --verify HEAD) &&
++ head12=$(git -C sm2 rev-parse --short --verify HEAD) &&
+ mv sm2 sm2-bak &&
cat >expected <<-EOF &&
Submodule sm1 $head7...0000000 (submodule deleted)
3: e1c7c2f63e ! 2: a438f4072d tests: Use `git submodule add` instead of `git add`
@@ Commit message
Signed-off-by: Josh Steadmon <steadmon@google.com>
Signed-off-by: Calvin Wan <calvinwan@google.com>
- ## t/t0008-ignores.sh ##
-@@ t/t0008-ignores.sh: test_expect_success 'setup' '
- git add a &&
- git commit -m"commit in submodule"
- ) &&
-- git add a/submodule &&
-+ git submodule add ./a/submodule ./a/submodule &&
- cat <<-\EOF >.gitignore &&
- one
- ignored-*
-
## t/t2013-checkout-submodule.sh ##
@@ t/t2013-checkout-submodule.sh: test_expect_success 'setup' '
(cd submodule &&
@@ t/t2103-update-index-ignore-missing.sh: test_expect_success basics '
git commit -m "sub initial"
) &&
- git add xyzzy &&
-+ git submodule add ./xyzzy &&
++ git add ./xyzzy &&
test_tick &&
git commit -m initial &&
@@ t/t4020-diff-external.sh: test_expect_success 'clean up crlf leftovers' '
git commit -m "add submodule" &&
( cd sub && test_commit sub2 ) &&
- ## t/t4035-diff-quiet.sh ##
-@@ t/t4035-diff-quiet.sh: test_expect_success 'git diff-index --cached HEAD^' '
- test_expect_success 'git diff-index --cached HEAD^' '
- echo text >>b &&
- echo 3 >c &&
-+ git submodule add ./test-outside/repo ./test-outside/repo &&
- git add . &&
- test_expect_code 1 git diff-index --quiet --cached HEAD^ >cnt &&
- test_line_count = 0 cnt
-
## t/t5531-deep-submodule-push.sh ##
@@ t/t5531-deep-submodule-push.sh: test_expect_success setup '
git add junk &&
@@ t/t6416-recursive-corner-cases.sh: test_expect_success 'setup conflicting entry
git checkout -b C A &&
- ## t/t6430-merge-recursive.sh ##
-@@ t/t6430-merge-recursive.sh: test_expect_success 'merging with triple rename across D/F conflict' '
- echo content3 >sub2/file3 &&
- mkdir simple &&
- echo base >simple/bar &&
-+ git -c protocol.file.allow=always submodule add ./sym &&
- git add -A &&
- test_tick &&
- git commit -m base &&
-
## t/t6437-submodule-merge.sh ##
@@ t/t6437-submodule-merge.sh: test_expect_success setup '
git add file &&
4: a7630f5a4e ! 3: 336e095b38 tests: use `git submodule add` and fix expected diffs
@@ t/t4041-diff-submodule-option.sh: test_expect_success 'typechanged submodule(sub
diff --git a/sm1 b/sm1
new file mode 100644
@@ t/t4041-diff-submodule-option.sh: test_expect_success 'nonexistent commit' '
- fullhead6=$(cd sm1 && git rev-parse --verify HEAD) &&
+ fullhead6=$(git -C sm1 rev-parse --verify HEAD) &&
git diff-index -p --submodule=log HEAD >actual &&
cat >expected <<-EOF &&
+ diff --git a/.gitmodules b/.gitmodules
@@ t/t4041-diff-submodule-option.sh: test_expect_success 'given commit' '
Submodule sm2 0000000...$head7 (new submodule)
EOF
@@ t/t4041-diff-submodule-option.sh: test_expect_success 'given commit --submodule=short' '
- fullhead7=$(cd sm2 && git rev-parse --verify HEAD) &&
+ fullhead7=$(git -C sm2 rev-parse --verify HEAD) &&
git diff-index -p --submodule=short HEAD^ >actual &&
cat >expected <<-EOF &&
+ diff --git a/.gitmodules b/.gitmodules
5: 7182e1fb2f = 4: 40a4647ae3 tests: use `git submodule add` and fix expected status
-: ---------- > 5: caa2f228a0 tests: remove duplicate .gitmodules path
6: 6d2ef61fb4 ! 6: 28cb291edd add: reject nested repositories
@@ Commit message
Due to this experience, we believe that Git's default behavior should be
changed to disallow adding embedded repositories. This commit changes
- the existing warning into a fatal error while preserving the
- `--no-warn-embedded-repo` flag as a way to bypass this check.
+ the existing warning into a fatal error, rewrites the advice given, and
+ deprecates `--no-warn-embedded-repo` in favor of `--allow-embedded-repo`
+ to bypass the fatal error.
Signed-off-by: Josh Steadmon <steadmon@google.com>
Signed-off-by: Calvin Wan <calvinwan@google.com>
## Documentation/git-add.txt ##
@@ Documentation/git-add.txt: for "git add --no-all <pathspec>...", i.e. ignored removed files.
+ be ignored, no matter if they are already present in the work
tree or not.
- --no-warn-embedded-repo::
+---no-warn-embedded-repo::
- By default, `git add` will warn when adding an embedded
++--allow-embedded-repo::
+ By default, `git add` will error out when adding an embedded
repository to the index without using `git submodule add` to
- create an entry in `.gitmodules`. This option will suppress the
- warning (e.g., if you are manually performing operations on
+- submodules).
+ create an entry in `.gitmodules`. This option will allow the
-+ embedded repository to be added and suppress the error.
-+ (e.g., if you are manually performing operations on
- submodules).
++ embedded repository to be added. (e.g., if you are manually
++ performing operations on submodules).
++
++--no-warn-embedded-repo::
++ This option is deprecated in favor of '--add-embedded-repo'.
++ Passing this option still suppresses advice but does not bypass
++ the error.
--renormalize::
+ Apply the "clean" process freshly to all tracked files to
## builtin/add.c ##
-@@ builtin/add.c: static const char embedded_advice[] = N_(
+@@ builtin/add.c: N_("The following paths are ignored by one of your .gitignore files:\n");
+ static int verbose, show_only, ignored_too, refresh_only;
+ static int ignore_add_errors, intent_to_add, ignore_missing;
+ static int warn_on_embedded_repo = 1;
++static int allow_embedded_repo = 0;
+
+ #define ADDREMOVE_DEFAULT 1
+ static int addremove = ADDREMOVE_DEFAULT;
+@@ builtin/add.c: static struct option builtin_add_options[] = {
+ N_("override the executable bit of the listed files")),
+ OPT_HIDDEN_BOOL(0, "warn-embedded-repo", &warn_on_embedded_repo,
+ N_("warn when adding an embedded repository")),
++ OPT_HIDDEN_BOOL(0, "allow-embedded-repo", &allow_embedded_repo,
++ N_("allow adding an embedded repository")),
+ OPT_PATHSPEC_FROM_FILE(&pathspec_from_file),
+ OPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),
+ OPT_END(),
+@@ builtin/add.c: static int add_config(const char *var, const char *value, void *cb)
+ }
+
+ static const char embedded_advice[] = N_(
+-"You've added another git repository inside your current repository.\n"
++"You attempted to add another git repository inside your current repository.\n"
+ "Clones of the outer repository will not contain the contents of\n"
+ "the embedded repository and will not know how to obtain it.\n"
+ "If you meant to add a submodule, use:\n"
"\n"
- " git rm --cached %s\n"
+ " git submodule add <url> %s\n"
"\n"
--"See \"git help submodule\" for more information."
+-"If you added this path by mistake, you can remove it from the\n"
+-"index with:\n"
+"See \"git help submodule\" for more information.\n"
-+"\n"
+ "\n"
+-" git rm --cached %s\n"
+"If you cannot use submodules, you may bypass this check with:\n"
-+"\n"
-+" git add --no-warn-embedded-repo %s\n"
+ "\n"
+-"See \"git help submodule\" for more information."
++" git add --allow-embedded-repo %s\n"
);
-static void check_embedded_repo(const char *path)
@@ builtin/add.c: static const char embedded_advice[] = N_(
struct strbuf name = STRBUF_INIT;
static int adviced_on_embedded_repo = 0;
- if (!warn_on_embedded_repo)
+- if (!warn_on_embedded_repo)
- return;
++ if (allow_embedded_repo)
+ goto cleanup;
if (!ends_with(path, "/"))
- return;
@@ builtin/add.c: static const char embedded_advice[] = N_(
- warning(_("adding embedded git repository: %s"), name.buf);
+ error(_("cannot add embedded git repository: %s"), name.buf);
if (!adviced_on_embedded_repo &&
- advice_enabled(ADVICE_ADD_EMBEDDED_REPO)) {
-- advise(embedded_advice, name.buf, name.buf);
-+ advise(embedded_advice, name.buf, name.buf, name.buf);
+- advice_enabled(ADVICE_ADD_EMBEDDED_REPO)) {
++ warn_on_embedded_repo &&
++ advice_enabled(ADVICE_ADD_EMBEDDED_REPO)) {
+ advise(embedded_advice, name.buf, name.buf);
adviced_on_embedded_repo = 1;
}
@@ builtin/add.c: static int add_files(struct dir_struct *dir, int flags)
advise_on_updating_sparse_paths(&matched_sparse_paths);
exit_status = 1;
+ ## builtin/submodule--helper.c ##
+@@ builtin/submodule--helper.c: static void configure_added_submodule(struct add_data *add_data)
+
+ add_submod.git_cmd = 1;
+ strvec_pushl(&add_submod.args, "add",
+- "--no-warn-embedded-repo", NULL);
++ "--allow-embedded-repo", NULL);
+ if (add_data->force)
+ strvec_push(&add_submod.args, "--force");
+ strvec_pushl(&add_submod.args, "--", add_data->sm_path, NULL);
+@@ builtin/submodule--helper.c: static int module_add(int argc, const char **argv, const char *prefix)
+ cp.git_cmd = 1;
+ cp.no_stdout = 1;
+ strvec_pushl(&cp.args, "add", "--dry-run", "--ignore-missing",
+- "--no-warn-embedded-repo", add_data.sm_path, NULL);
++ "--allow-embedded-repo", add_data.sm_path, NULL);
+ if ((ret = pipe_command(&cp, NULL, 0, NULL, 0, &sb, 0))) {
+ strbuf_complete_line(&sb);
+ fputs(sb.buf, stderr);
+
+ ## t/t0008-ignores.sh ##
+@@ t/t0008-ignores.sh: test_expect_success 'setup' '
+ git add a &&
+ git commit -m"commit in submodule"
+ ) &&
+- git add a/submodule &&
++ git add --allow-embedded-repo a/submodule &&
+ cat <<-\EOF >.gitignore &&
+ one
+ ignored-*
+
+ ## t/t2103-update-index-ignore-missing.sh ##
+@@ t/t2103-update-index-ignore-missing.sh: test_expect_success basics '
+ git add file &&
+ git commit -m "sub initial"
+ ) &&
+- git add ./xyzzy &&
++ git add --allow-embedded-repo ./xyzzy &&
+
+ test_tick &&
+ git commit -m initial &&
+
+ ## t/t4035-diff-quiet.sh ##
+@@ t/t4035-diff-quiet.sh: test_expect_success 'git diff-index --cached HEAD^' '
+ test_expect_success 'git diff-index --cached HEAD^' '
+ echo text >>b &&
+ echo 3 >c &&
+- git add . &&
++ git add --allow-embedded-repo . &&
+ test_expect_code 1 git diff-index --quiet --cached HEAD^ >cnt &&
+ test_line_count = 0 cnt
+ '
+
+ ## t/t6430-merge-recursive.sh ##
+@@ t/t6430-merge-recursive.sh: test_expect_success 'merging with triple rename across D/F conflict' '
+ echo content3 >sub2/file3 &&
+ mkdir simple &&
+ echo base >simple/bar &&
+- git add -A &&
++ git add -A --allow-embedded-repo &&
+ test_tick &&
+ git commit -m base &&
+
+
## t/t7400-submodule-basic.sh ##
@@ t/t7400-submodule-basic.sh: test_expect_success 'setup - repository in init subdirectory' '
test_expect_success 'setup - commit with gitlink' '
echo a >a &&
echo z >z &&
- git add a init z &&
-+ git add --no-warn-embedded-repo a init z &&
++ git add --allow-embedded-repo a init z &&
git commit -m "super commit 1"
'
@@ t/t7400-submodule-basic.sh: test_expect_success 'set up for relative path tests'
test_commit foo
) &&
- git add sub &&
-+ git add --no-warn-embedded-repo sub &&
++ git add --allow-embedded-repo sub &&
git config -f .gitmodules submodule.sub.path sub &&
git config -f .gitmodules submodule.sub.url ../subrepo &&
cp .git/config pristine-.git-config &&
@@ t/t7412-submodule-absorbgitdirs.sh: test_expect_success 'absorb the git dir in a
git init sub2 &&
test_commit -C sub2 first &&
- git add sub2 &&
-+ git add --no-warn-embedded-repo sub2 &&
++ git add --allow-embedded-repo sub2 &&
git commit -m superproject
'
@@ t/t7414-submodule-mistakes.sh: test_expect_success 'create embedded repository'
'
-test_expect_success '--no-warn-embedded-repo suppresses warning' '
-+test_expect_success '--no-warn-embedded-repo suppresses error message' '
++test_expect_success '--allow-embedded-repo adds embedded repository and suppresses error message' '
test_when_finished "git rm --cached -f embed" &&
- git add --no-warn-embedded-repo embed 2>stderr &&
+- git add --no-warn-embedded-repo embed 2>stderr &&
- test_i18ngrep ! warning stderr
++ git add --allow-embedded-repo embed 2>stderr &&
+ test_i18ngrep ! fatal stderr
++'
++
++test_expect_success '--no-warn-embedded-repo dies and suppresses advice' '
++ test_must_fail git add --no-warn-embedded-repo embed 2>stderr &&
++ test_i18ngrep ! hint stderr &&
++ test_i18ngrep fatal stderr
'
-test_expect_success 'no warning when updating entry' '
+test_expect_success 'no error message when updating entry' '
test_when_finished "git rm --cached -f embed" &&
- git add embed &&
-+ git add --no-warn-embedded-repo embed &&
++ git add --allow-embedded-repo embed &&
git -C embed commit --allow-empty -m two &&
git add embed 2>stderr &&
- test_i18ngrep ! warning stderr
@@ t/t7414-submodule-mistakes.sh: test_expect_success 'create embedded repository'
'
-test_expect_success 'submodule add does not warn' '
-+test_expect_success 'submodule add does not issue error message' '
++test_expect_success 'submodule add neither fails nor issues error message' '
test_when_finished "git rm -rf submodule .gitmodules" &&
git -c protocol.file.allow=always \
submodule add ./embed submodule 2>stderr &&
@@ t/t7450-bad-git-dotfiles.sh: test_expect_success 'git dirs of sibling submodules
git clone . thing1 &&
git clone . thing2 &&
- git add .gitmodules thing1 thing2 &&
-+ git add --no-warn-embedded-repo .gitmodules thing1 thing2 &&
++ git add --allow-embedded-repo .gitmodules thing1 thing2 &&
test_tick &&
git commit -m nested
) &&
--
2.39.2.722.g9855ee24e9-goog
next prev parent reply other threads:[~2023-02-28 18:53 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-13 18:21 [RFC PATCH 0/6] add: block invalid submodules Calvin Wan
2023-02-13 18:21 ` [RFC PATCH 1/6] leak fix: cache_put_path Calvin Wan
2023-02-13 19:23 ` Junio C Hamano
2023-02-14 19:56 ` Calvin Wan
2023-02-14 21:08 ` Junio C Hamano
2023-02-14 21:39 ` Calvin Wan
2023-02-14 21:59 ` Junio C Hamano
2023-02-13 18:21 ` [RFC PATCH 2/6] t4041, t4060: modernize test style Calvin Wan
2023-02-13 19:41 ` Junio C Hamano
2023-02-14 20:22 ` Calvin Wan
2023-02-13 18:21 ` [RFC PATCH 3/6] tests: Use `git submodule add` instead of `git add` Calvin Wan
2023-02-13 18:21 ` [RFC PATCH 4/6] tests: use `git submodule add` and fix expected diffs Calvin Wan
2023-02-13 23:07 ` Junio C Hamano
2023-02-13 23:19 ` Junio C Hamano
2023-02-13 18:21 ` [RFC PATCH 5/6] tests: use `git submodule add` and fix expected status Calvin Wan
2023-02-13 18:21 ` [RFC PATCH 6/6] add: reject nested repositories Calvin Wan
2023-02-13 20:42 ` Jeff King
2023-02-14 2:17 ` Junio C Hamano
2023-02-14 16:07 ` Jeff King
2023-02-14 16:32 ` Junio C Hamano
2023-02-14 21:45 ` Calvin Wan
2023-02-28 18:52 ` Calvin Wan [this message]
2023-02-28 18:56 ` [PATCH v2 1/6] t4041, t4060: modernize test style Calvin Wan
2023-03-06 19:32 ` Glen Choo
2023-03-06 20:40 ` Calvin Wan
2023-02-28 18:56 ` [PATCH v2 2/6] tests: Use `git submodule add` instead of `git add` Calvin Wan
2023-02-28 23:30 ` Junio C Hamano
2023-03-03 0:16 ` Calvin Wan
2023-03-06 21:26 ` Glen Choo
2023-02-28 18:56 ` [PATCH v2 3/6] tests: use `git submodule add` and fix expected diffs Calvin Wan
2023-03-06 23:34 ` Glen Choo
2023-03-06 23:57 ` Junio C Hamano
2023-02-28 18:56 ` [PATCH v2 4/6] tests: use `git submodule add` and fix expected status Calvin Wan
2023-03-07 0:15 ` Glen Choo
2023-02-28 18:56 ` [PATCH v2 5/6] tests: remove duplicate .gitmodules path Calvin Wan
2023-02-28 23:35 ` Junio C Hamano
2023-03-02 23:09 ` Calvin Wan
2023-03-07 0:51 ` Glen Choo
2023-02-28 18:56 ` [PATCH v2 6/6] add: reject nested repositories Calvin Wan
2023-03-07 2:04 ` Glen Choo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230228185253.2356546-1-calvinwan@google.com \
--to=calvinwan@google.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
--cc=steadmon@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.