Re: [PATCHv3] fast-import: tighten parsing of mark references

[Jonathan Nieder <jrnieder@gmail.com>]

Pete Wyckoff wrote:

> This addresses all of Jonathan's comments, in particular:

Nice.  Thanks much.  I only have a few small worries left:

[...]
> +++ b/t/t9300-fast-import.sh
> @@ -2635,4 +2635,280 @@ test_expect_success \
[...]
> +test_expect_success 'S: filemodify with garbage after sha1 must fail' '
> +	sha1=$(grep -w :103 marks | cut -d\  -f2) &&

"grep -w" isn't used elsewhere in the testsuite.  Is it portable?

[...]
> +# inline is misspelled; fast-import thinks it is some unknown dataref
> +# and complains "Invalid SHA1"
> +test_expect_success 'S: notemodify with garbage after inline dataref must fail' '
> +	test_must_fail git fast-import --import-marks=marks <<-EOF 2>err &&
> +	commit refs/heads/S
> +	committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
> +	data <<COMMIT
> +	commit S note dataref inline
> +	COMMIT
> +	N inlineX :2
> +	data <<BLOB
> +	note blob
> +	BLOB
> +	EOF
> +	cat err &&
> +	test_i18ngrep "nvalid SHA1" err
> +'

If I understood the discussion before correctly, this error message is
suboptimal and something like "invalid dataref" would be a little
clearer, right?

That's orthogonal to what this patch is about so I'm not suggesting
changing it.  But shouldn't the test just check that fast-import fails
without committing to any particular message?

Cheers,
Jonathan