Re: [PATCH 2/7] t: introduce tests for unexpected object types

[Taylor Blau <me@ttaylorr.com>]

Hi Eric,

On Mon, Apr 08, 2019 at 11:28:19PM -0400, Eric Sunshine wrote:
> On Mon, Apr 8, 2019 at 10:31 PM Taylor Blau <me@ttaylorr.com> wrote:
> > On Mon, Apr 08, 2019 at 03:44:25PM +0900, Junio C Hamano wrote:
> > > Eric Sunshine <sunshine@sunshineco.com> writes:
> > > >> > > +   git cat-file commit $commit |
> > > >> > > +           perl -lpe "/^author/ && print q(parent $blob)" \
> > > >> > > +           >broken-commit &&
> > > >
> > > > Trivial and portable 'sed' equivalent:
> > > >
> > > > git cat-file commit $commit | sed "/^author/ { h; s/.*/parent $blob/; G; }"
> > >
> > > Looks good.  I had a bit of head scratching moment when I saw that
> > > "perl -lpe" one-liner; this sed expression may not be crystal clear
> > > to those who are not used to, but it is not so bad, either.
> >
> > Should I take this as your endorsement of putting 'git' on the left-hand
> > side of a pipe? ;-).
>
> I suspect that Junio's "Looks good" was referring to the 'sed expression.

I think that you are right -- and I'll happily _not_ introduce more Git
on the left-hand-side of a pipe instances.

I noticed a few more instances in t6102 where we do something similar
to:

  git cat-file -p <something> | sed ... >broken-<something> &&

I wrote the following patch, which I've folded into my local copy (and
will send with v2):

diff --git a/t/t6102-rev-list-unexpected-objects.sh b/t/t6102-rev-list-unexpected-objects.sh
index b9d82f9542..15072ecce3 100755
--- a/t/t6102-rev-list-unexpected-objects.sh
+++ b/t/t6102-rev-list-unexpected-objects.sh
@@ -7,7 +7,8 @@ test_description='git rev-list should handle unexpected object types'
 test_expect_success 'setup well-formed objects' '
 	blob="$(printf "foo" | git hash-object -w --stdin)" &&
 	tree="$(printf "100644 blob $blob\tfoo" | git mktree)" &&
-	commit="$(git commit-tree $tree -m "first commit")"
+	commit="$(git commit-tree $tree -m "first commit")" &&
+	git cat-file commit $commit >good-commit
 '

 test_expect_success 'setup unexpected non-blob entry' '
@@ -37,8 +38,8 @@ test_expect_failure 'traverse unexpected non-tree entry (seen)' '
 '

 test_expect_success 'setup unexpected non-commit parent' '
-	git cat-file commit $commit | \
-		sed "/^author/ { h; s/.*/parent $blob/; G; }" >broken-commit &&
+	sed "/^author/ { h; s/.*/parent $blob/; G; }" <good-commit \
+		>broken-commit &&
 	broken_commit="$(git hash-object -w --literally -t commit \
 		broken-commit)"
 '
@@ -55,8 +56,7 @@ test_expect_success 'traverse unexpected non-commit parent (seen)' '
 '

 test_expect_success 'setup unexpected non-tree root' '
-	git cat-file commit $commit |
-	sed -e "s/$tree/$blob/" >broken-commit &&
+	sed -e "s/$tree/$blob/" <good-commit >broken-commit &&
 	broken_commit="$(git hash-object -w --literally -t commit \
 		broken-commit)"
 '
@@ -71,8 +71,9 @@ test_expect_failure 'traverse unexpected non-tree root (seen)' '

 test_expect_success 'setup unexpected non-commit tag' '
 	git tag -a -m "tagged commit" tag $commit &&
+	git cat-file tag tag >good-tag &&
 	test_when_finished "git tag -d tag" &&
-	git cat-file -p tag | sed -e "s/$commit/$blob/" >broken-tag &&
+	sed -e "s/$commit/$blob/" <good-tag >broken-tag &&
 	tag=$(git hash-object -w --literally -t tag broken-tag)
 '

@@ -87,9 +88,9 @@ test_expect_success 'traverse unexpected non-commit tag (seen)' '

 test_expect_success 'setup unexpected non-tree tag' '
 	git tag -a -m "tagged tree" tag $tree &&
+	git cat-file tag tag >good-tag &&
 	test_when_finished "git tag -d tag" &&
-	git cat-file -p tag |
-	sed -e "s/$tree/$blob/" >broken-tag &&
+	sed -e "s/$tree/$blob/" <good-tag >broken-tag &&
 	tag=$(git hash-object -w --literally -t tag broken-tag)
 '

@@ -104,9 +105,9 @@ test_expect_success 'traverse unexpected non-tree tag (seen)' '

 test_expect_success 'setup unexpected non-blob tag' '
 	git tag -a -m "tagged blob" tag $blob &&
+	git cat-file tag tag >good-tag &&
 	test_when_finished "git tag -d tag" &&
-	git cat-file -p tag |
-	sed -e "s/$blob/$commit/" >broken-tag &&
+	sed -e "s/$blob/$commit/" <good-tag >broken-tag &&
 	tag=$(git hash-object -w --literally -t tag broken-tag)
 '

> With all the recent work of moving away from having Git upstream of a
> pipe, let's not intentionally introduce a new instance. I wrote the
> example 'sed' expression that way merely to mirror how the original
> 'perl' version was written to make it easier to see the equivalence
> (not because it was intended as an endorsement of having Git upstream
> of a pipe).

I see, and thank you for the clarification. Let me know if you like the
patch above.

Thanks,
Taylor