On 11/07/18 20:31, Ramsay Jones wrote:

On 07/07/18 02:32, Jeff King wrote:

[snip]

quoted

Hmm, we seem to have "info" these days, so maybe that would do what I
want. I.e., I wonder if the patch below does everything we'd want. It's
late here and I probably won't get back to this until Monday, but you
may want to play with it in the meantime.

Sorry, I've been busy with other things and have not had the
time to try the patch below (still trying to catch up with
the mailing-list emails!).

quoted

diff --git a/fsck.c b/fsck.c
index 48e7e36869..0b0003055e 100644
--- a/fsck.c
+++ b/fsck.c

@@ -61,7 +61,6 @@ static struct oidset gitmodules_done = OIDSET_INIT;
 	FUNC(ZERO_PADDED_DATE, ERROR) \
 	FUNC(GITMODULES_MISSING, ERROR) \
 	FUNC(GITMODULES_BLOB, ERROR) \
-	FUNC(GITMODULES_PARSE, ERROR) \
 	FUNC(GITMODULES_NAME, ERROR) \
 	FUNC(GITMODULES_SYMLINK, ERROR) \
 	/* warnings */ \

@@ -76,6 +75,7 @@ static struct oidset gitmodules_done = OIDSET_INIT;
 	FUNC(NUL_IN_COMMIT, WARN) \
 	/* infos (reported as warnings, but ignored by default) */ \
 	FUNC(BAD_TAG_NAME, INFO) \
+	FUNC(GITMODULES_PARSE, INFO) \
 	FUNC(MISSING_TAGGER_ENTRY, INFO)
 
 #define MSG_ID(id, msg_type) FSCK_MSG_##id,

So, just squinting at this in my email client, if this allowed
a push/fetch to succeed (along with an 'info' message), while
providing an admin the means to configure it to loudly deny
the push/fetch - then I think we have a winner! ;-)

Sorry for not testing the patch.

OK, so I found some time to test this tonight. It is not good
news (assuming that I haven't messed up the testing, of course). :(

On top of 'pu' (@9026cfc855), I reverted commit d4c5675233
("fsck: silence stderr when parsing .gitmodules", 2018-06-28) and
added the patch given below.

Unfortunately, the final test fails, thus:

  $ cd t
  $ ./t7415-submodule-names.sh
  ok 1 - check names
  ok 2 - create innocent subrepo
  ok 3 - submodule add refuses invalid names
  ok 4 - add evil submodule
  ok 5 - add other submodule
  ok 6 - clone evil superproject
  ok 7 - fsck detects evil superproject
  ok 8 - transfer.fsckObjects detects evil superproject (unpack)
  ok 9 - transfer.fsckObjects detects evil superproject (index)
  ok 10 - create oddly ordered pack
  ok 11 - transfer.fsckObjects handles odd pack (unpack)
  ok 12 - transfer.fsckObjects handles odd pack (index)
  ok 13 - index-pack --strict works for non-repo pack
  ok 14 - fsck detects symlinked .gitmodules file
  ok 15 - fsck detects non-blob .gitmodules
  ok 16 - fsck detects corrupt .gitmodules
  ok 17 - push warns about corrupt .gitmodules
  not ok 18 - push rejects corrupt .gitmodules (policy)
  #	
  #		rm -rf dst.git &&
  #		git init --bare dst.git &&
  #		git -C dst.git config transfer.fsckObjects true &&
  #		git -C dst.git config fsck.gitmodulesParse error &&
  #		test_must_fail git -C corrupt push ../dst.git HEAD 2>output &&
  #		grep gitmodulesParse output &&
  #		test_i18ngrep ! "bad config" output
  #	
  # failed 1 among 18 test(s)
  1..18
  $ 

i.e. the test_must_fail doesn't! ;-)

I have to go out now, but I will hopefully take a look at
this again tomorrow. (Do the test additions look OK?)

ATB,
Ramsay Jones

-- >8 --
Subject: [PATCH] WIP: try jeff's last patch

Signed-off-by: Ramsay Jones <redacted>
---
 fsck.c                     |  2 +-
 t/t7415-submodule-names.sh | 26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/fsck.c b/fsck.c
index 17b3a51fa..b74856cee 100644
--- a/fsck.c
+++ b/fsck.c

@@ -64,7 +64,6 @@ static struct oidset gitmodules_done = OIDSET_INIT;
 	FUNC(ZERO_PADDED_DATE, ERROR) \
 	FUNC(GITMODULES_MISSING, ERROR) \
 	FUNC(GITMODULES_BLOB, ERROR) \
-	FUNC(GITMODULES_PARSE, ERROR) \
 	FUNC(GITMODULES_NAME, ERROR) \
 	FUNC(GITMODULES_SYMLINK, ERROR) \
 	/* warnings */ \

@@ -79,6 +78,7 @@ static struct oidset gitmodules_done = OIDSET_INIT;
 	FUNC(NUL_IN_COMMIT, WARN) \
 	/* infos (reported as warnings, but ignored by default) */ \
 	FUNC(BAD_TAG_NAME, INFO) \
+	FUNC(GITMODULES_PARSE, INFO) \
 	FUNC(MISSING_TAGGER_ENTRY, INFO)
 
 #define MSG_ID(id, msg_type) FSCK_MSG_##id,

diff --git a/t/t7415-submodule-names.sh b/t/t7415-submodule-names.sh
index ba8af785a..ef9535a44 100755
--- a/t/t7415-submodule-names.sh
+++ b/t/t7415-submodule-names.sh

@@ -185,10 +185,36 @@ test_expect_success 'fsck detects corrupt .gitmodules' '
 		git add .gitmodules &&
 		git commit -m "broken gitmodules" &&
 
+		# issues an "info" warning, but does not fail
+		git fsck 2>output &&
+		grep gitmodulesParse output &&
+		test_i18ngrep ! "bad config" output &&
+
+		# up-rate gitmodulesParse to error
+		git config fsck.gitmodulesParse error &&
 		test_must_fail git fsck 2>output &&
 		grep gitmodulesParse output &&
 		test_i18ngrep ! "bad config" output
 	)
 '
 
+test_expect_success 'push warns about corrupt .gitmodules' '
+	rm -rf dst.git &&
+	git init --bare dst.git &&
+	git -C dst.git config transfer.fsckObjects true &&
+	git -C corrupt push ../dst.git HEAD 2>output &&
+	grep gitmodulesParse output &&
+	test_i18ngrep ! "bad config" output
+'
+
+test_expect_success 'push rejects corrupt .gitmodules (policy)' '
+	rm -rf dst.git &&
+	git init --bare dst.git &&
+	git -C dst.git config transfer.fsckObjects true &&
+	git -C dst.git config fsck.gitmodulesParse error &&
+	test_must_fail git -C corrupt push ../dst.git HEAD 2>output &&
+	grep gitmodulesParse output &&
+	test_i18ngrep ! "bad config" output
+'
+
 test_done

-- 
2.18.0