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@xxxxxxxxxxxx> wrote:
> > On Mon, Apr 08, 2019 at 03:44:25PM +0900, Junio C Hamano wrote:
> > > Eric Sunshine <sunshine@xxxxxxxxxxxxxx> 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
