{"thread":{"id":"14828","subject":"[git/perl] unusual syntax?","startedAt":"2008-08-04T04:49:27Z","lastAt":"2008-08-05T06:12:01Z","messageCount":9,"participants":["Ray Chuan","Abhijit Menon-Sen","Petr Baudis","Junio C Hamano","David Christensen"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"86147","messageId":"be6fef0d0808032149p651309a8o773dca5f16923ee1@mail.gmail.com","threadId":"14828","inReplyTo":null,"subject":"[git/perl] unusual syntax?","fromName":"Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2008-08-04T04:49:27Z","receivedAt":"2008-08-04T04:49:27Z","isPatch":false,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\ni noticed that this doesn't work for me (Perl 5.10):\n\nsub _close_hash_and_insert_object {\n\tmy ($self) = @_;\n\n\treturn unless defined($self->{hash_object_pid});\n\n\tmy @vars = map { 'hash_object_' . $_ } qw(pid in out ctx);\n\n\tcommand_close_bidi_pipe($self->{@vars});\n\tdelete $self->{@vars};\n}\n\n\n$self->{@vars} evaluates to undef. i can't find any mention of using\narrays to dereference objects in the manual and elsewhere; is this a\nmistake?\n\n-- \nCheers,\nRay Chuan\n"},{"id":"86149","messageId":"20080804050247.GA13539@toroid.org","threadId":"14828","inReplyTo":"be6fef0d0808032149p651309a8o773dca5f16923ee1@mail.gmail.com","subject":"[PATCH] Fix hash slice syntax error","fromName":"Abhijit Menon-Sen","fromEmail":"ams@toroid.org","sentAt":"2008-08-04T05:02:47Z","receivedAt":"2008-08-04T05:02:47Z","isPatch":true,"sender":{"key":"ams@toroid.org","avatar":null},"body":"\nSigned-off-by: Abhijit Menon-Sen <ams@toroid.org>\n---\n\nAt 2008-08-04 12:49:27 +0800, rctay89@gmail.com wrote:\n>\n> $self->{@vars} evaluates to undef. i can't find any mention of using\n> arrays to dereference objects in the manual and elsewhere; is this a\n> mistake?\n\nYes, @vars would be interpreted in scalar context, which certainly isn't\nthe intended effect.\n\n-- ams\n\n perl/Git.pm |    8 ++++----\n 1 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex 087d3d0..2ef437f 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -839,8 +839,8 @@ sub _close_hash_and_insert_object {\n \n \tmy @vars = map { 'hash_object_' . $_ } qw(pid in out ctx);\n \n-\tcommand_close_bidi_pipe($self->{@vars});\n-\tdelete $self->{@vars};\n+\tcommand_close_bidi_pipe(@$self{@vars});\n+\tdelete @$self{@vars};\n }\n \n =item cat_blob ( SHA1, FILEHANDLE )\n@@ -928,8 +928,8 @@ sub _close_cat_blob {\n \n \tmy @vars = map { 'cat_blob_' . $_ } qw(pid in out ctx);\n \n-\tcommand_close_bidi_pipe($self->{@vars});\n-\tdelete $self->{@vars};\n+\tcommand_close_bidi_pipe(@$self{@vars});\n+\tdelete @$self{@vars};\n }\n \n =back\n-- \n1.6.0.rc0.43.g2aa74\n"},{"id":"86156","messageId":"20080804075313.21325.28396.stgit@localhost","threadId":"14828","inReplyTo":"be6fef0d0808032149p651309a8o773dca5f16923ee1@mail.gmail.com","subject":"[PATCH] Git.pm: Fix internal git_command_bidi_pipe() users","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2008-08-04T07:56:04Z","receivedAt":"2008-08-04T07:56:04Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"The hash_and_insert_object() and cat_blob() helpers were using\nan incorrect slice-from-ref Perl syntax. This patch fixes that up\nin the _close_*() helpers and make the _open_*() helpers use the\nsame syntax for consistnecy.\n\nSigned-off-by: Petr Baudis <pasky@suse.cz>\n---\n\n  Wow, the command_bidi_pipe API really is dirty. Of course, it is\nmy fault as anyone's since I didn't get around to review the patches\nintroducing it.\n\n perl/Git.pm |   16 ++++++----------\n 1 files changed, 6 insertions(+), 10 deletions(-)\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex 087d3d0..0624428 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -827,8 +827,7 @@ sub _open_hash_and_insert_object_if_needed {\n \n \treturn if defined($self->{hash_object_pid});\n \n-\t($self->{hash_object_pid}, $self->{hash_object_in},\n-\t $self->{hash_object_out}, $self->{hash_object_ctx}) =\n+\t@$self{map { \"hash_object_$_\" } qw(pid in out ctx)} =\n \t\tcommand_bidi_pipe(qw(hash-object -w --stdin-paths));\n }\n \n@@ -837,9 +836,8 @@ sub _close_hash_and_insert_object {\n \n \treturn unless defined($self->{hash_object_pid});\n \n-\tmy @vars = map { 'hash_object_' . $_ } qw(pid in out ctx);\n-\n-\tcommand_close_bidi_pipe($self->{@vars});\n+\tmy @vars = map { \"hash_object_$_\" } qw(pid in out ctx);\n+\tcommand_close_bidi_pipe(@$self{@vars});\n \tdelete $self->{@vars};\n }\n \n@@ -916,8 +914,7 @@ sub _open_cat_blob_if_needed {\n \n \treturn if defined($self->{cat_blob_pid});\n \n-\t($self->{cat_blob_pid}, $self->{cat_blob_in},\n-\t $self->{cat_blob_out}, $self->{cat_blob_ctx}) =\n+\t@$self{map { \"cat_blob_$_\" } qw(pid in out ctx)} =\n \t\tcommand_bidi_pipe(qw(cat-file --batch));\n }\n \n@@ -926,9 +923,8 @@ sub _close_cat_blob {\n \n \treturn unless defined($self->{cat_blob_pid});\n \n-\tmy @vars = map { 'cat_blob_' . $_ } qw(pid in out ctx);\n-\n-\tcommand_close_bidi_pipe($self->{@vars});\n+\tmy @vars = map { \"cat_blob_$_\" } qw(pid in out ctx);\n+\tcommand_close_bidi_pipe(@$self{@vars});\n \tdelete $self->{@vars};\n }\n \n"},{"id":"86158","messageId":"7vtze12oij.fsf@gitster.siamese.dyndns.org","threadId":"14828","inReplyTo":"20080804075313.21325.28396.stgit@localhost","subject":"Re: [PATCH] Git.pm: Fix internal git_command_bidi_pipe() users","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-04T08:05:56Z","receivedAt":"2008-08-04T08:05:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Petr Baudis <pasky@suse.cz> writes:\n\n> The hash_and_insert_object() and cat_blob() helpers were using\n> an incorrect slice-from-ref Perl syntax. This patch fixes that up\n> in the _close_*() helpers and make the _open_*() helpers use the\n> same syntax for consistnecy.\n>\n> Signed-off-by: Petr Baudis <pasky@suse.cz>\n> ---\n>\n>   Wow, the command_bidi_pipe API really is dirty. Of course, it is\n> my fault as anyone's since I didn't get around to review the patches\n> introducing it.\n\nSorry, delete is still broken with your patch, isn't it?\n\nThe earlier patch from Abhijit Menon-Sen does this properly for\nclose_hash_and_insert and close_cat_blob, which I've queued already.\n"},{"id":"86160","messageId":"20080804082117.GI10151@machine.or.cz","threadId":"14828","inReplyTo":"7vtze12oij.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Git.pm: Fix internal git_command_bidi_pipe() users","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2008-08-04T08:21:17Z","receivedAt":"2008-08-04T08:21:17Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Mon, Aug 04, 2008 at 01:05:56AM -0700, Junio C Hamano wrote:\n> Petr Baudis <pasky@suse.cz> writes:\n> \n> > The hash_and_insert_object() and cat_blob() helpers were using\n> > an incorrect slice-from-ref Perl syntax. This patch fixes that up\n> > in the _close_*() helpers and make the _open_*() helpers use the\n> > same syntax for consistnecy.\n> >\n> > Signed-off-by: Petr Baudis <pasky@suse.cz>\n> > ---\n> >\n> >   Wow, the command_bidi_pipe API really is dirty. Of course, it is\n> > my fault as anyone's since I didn't get around to review the patches\n> > introducing it.\n> \n> Sorry, delete is still broken with your patch, isn't it?\n\nOh, right - I forgot that one and it didn't occur to me to test this\npart.\n\n> The earlier patch from Abhijit Menon-Sen does this properly for\n> close_hash_and_insert and close_cat_blob, which I've queued already.\n\nAbhijit, can you please tag your Git.pm patches so that I actually have\na chance to see and review it?\n\nThanks,\n\n\t\t\t\tPetr \"Pasky\" Baudis\n"},{"id":"86161","messageId":"7vhca12n2l.fsf@gitster.siamese.dyndns.org","threadId":"14828","inReplyTo":"20080804082117.GI10151@machine.or.cz","subject":"Re: [PATCH] Git.pm: Fix internal git_command_bidi_pipe() users","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-04T08:37:06Z","receivedAt":"2008-08-04T08:37:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Petr Baudis <pasky@suse.cz> writes:\n\n> On Mon, Aug 04, 2008 at 01:05:56AM -0700, Junio C Hamano wrote:\n>> Petr Baudis <pasky@suse.cz> writes:\n>> \n>> > The hash_and_insert_object() and cat_blob() helpers were using\n>> > an incorrect slice-from-ref Perl syntax. This patch fixes that up\n>> > in the _close_*() helpers and make the _open_*() helpers use the\n>> > same syntax for consistnecy.\n>> >\n>> > Signed-off-by: Petr Baudis <pasky@suse.cz>\n>> > ---\n>> >\n>> >   Wow, the command_bidi_pipe API really is dirty. Of course, it is\n>> > my fault as anyone's since I didn't get around to review the patches\n>> > introducing it.\n>> \n>> Sorry, delete is still broken with your patch, isn't it?\n>\n> Oh, right - I forgot that one and it didn't occur to me to test this\n> part.\n>\n>> The earlier patch from Abhijit Menon-Sen does this properly for\n>> close_hash_and_insert and close_cat_blob, which I've queued already.\n>\n> Abhijit, can you please tag your Git.pm patches so that I actually have\n> a chance to see and review it?\n\nIt is $gmane/91316, Message-ID: <20080804050247.GA13539@toroid.org>\n\nAfter queueing it, I actually had to revert it, because it seems to break\ngit-svn (t9106-git-svn-commit-diff-clobber.sh, test #8), and I am about to\ngo to bed.  If you did not see the same breakage with your patch that does\nnot \"fix\" delete, it could be that the git-svn uses some of the resources\nthat are released by the delete actually doing what it is asked to do.\n\n---\n\nAt 2008-08-04 12:49:27 +0800, rctay89@gmail.com wrote:\n>\n> $self->{@vars} evaluates to undef. i can't find any mention of using\n> arrays to dereference objects in the manual and elsewhere; is this a\n> mistake?\n\nYes, @vars would be interpreted in scalar context, which certainly isn't\nthe intended effect.\n\n-- ams\n\n perl/Git.pm |    8 ++++----\n 1 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex 087d3d0..2ef437f 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -839,8 +839,8 @@ sub _close_hash_and_insert_object {\n \n \tmy @vars = map { 'hash_object_' . $_ } qw(pid in out ctx);\n \n-\tcommand_close_bidi_pipe($self->{@vars});\n-\tdelete $self->{@vars};\n+\tcommand_close_bidi_pipe(@$self{@vars});\n+\tdelete @$self{@vars};\n }\n \n =item cat_blob ( SHA1, FILEHANDLE )\n@@ -928,8 +928,8 @@ sub _close_cat_blob {\n \n \tmy @vars = map { 'cat_blob_' . $_ } qw(pid in out ctx);\n \n-\tcommand_close_bidi_pipe($self->{@vars});\n-\tdelete $self->{@vars};\n+\tcommand_close_bidi_pipe(@$self{@vars});\n+\tdelete @$self{@vars};\n }\n \n =back\n-- \n1.6.0.rc0.43.g2aa74\n"},{"id":"86176","messageId":"20080804113827.GA1239@toroid.org","threadId":"14828","inReplyTo":"7vhca12n2l.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] Git.pm: localise $? in command_close_bidi_pipe()","fromName":"Abhijit Menon-Sen","fromEmail":"ams@toroid.org","sentAt":"2008-08-04T11:38:27Z","receivedAt":"2008-08-04T11:38:27Z","isPatch":true,"sender":{"key":"ams@toroid.org","avatar":null},"body":"Git::DESTROY calls _close_cat_blob and _close_hash_and_insert_object,\nwhich in turn call command_close_bidi_pipe, which calls waitpid, which\nalters $?. If this happens during global destruction, it may alter the\nprogram's exit status unexpectedly. Making $? local to the function\nsolves the problem.\n\n(The problem was discovered due to a failure of test #8 in\nt9106-git-svn-commit-diff-clobber.sh.)\n\nSigned-off-by: Abhijit Menon-Sen <ams@toroid.org>\n---\n\nAt 2008-08-04 01:37:06 -0700, gitster@pobox.com wrote:\n>\n> After queueing it, I actually had to revert it, because it seems to\n> break git-svn (t9106-git-svn-commit-diff-clobber.sh, test #8), and I\n> am about to go to bed.\n\nThis patch in addition to my earlier one should solve the problem.\n\nFor test #8 to fail, the \"git svn dcommit\" must succeed, but in both\ncases (i.e. without my patch applied, or with), the rebase fails:\n\n    rebase refs/remotes/git-svn: command returned error: 1\n\nThis results in a call to \"fatal $@\" on git-svn.perl:254, which calls\n\"exit 1\", and test_must_fail is happy.\n\nWith my patch, however, Git::DESTROY calls the two _close functions\nduring global destruction, which in turn call command_close_bidi_pipe,\nwhich calls waitpid with sensible arguments this time, which alters $?,\nthus altering the exit status of the dcommit itself to 0. Oops.\n\nAll of \"make test\" passes for me after this change.\n\n-- ams\n\n perl/Git.pm |    1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex 2ef437f..3b6707b 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -417,6 +417,7 @@ have more complicated structure.\n =cut\n \n sub command_close_bidi_pipe {\n+\tlocal $?;\n \tmy ($pid, $in, $out, $ctx) = @_;\n \tforeach my $fh ($in, $out) {\n \t\tunless (close $fh) {\n-- \n1.6.0.rc0.43.g2aa74\n"},{"id":"86201","messageId":"77D646CF-448B-434A-B969-653931B4A756@endpoint.com","threadId":"14828","inReplyTo":"be6fef0d0808032149p651309a8o773dca5f16923ee1@mail.gmail.com","subject":"Re: [git/perl] unusual syntax?","fromName":"David Christensen","fromEmail":"david@endpoint.com","sentAt":"2008-08-04T15:01:44Z","receivedAt":"2008-08-04T15:01:44Z","isPatch":false,"sender":{"key":"david@endpoint.com","avatar":"https://gravatar.com/avatar/6089b35cc409d9d15ab439753a213d7528cd5e0a04f3917fa452d8dc45296612?d=mp&s=160"},"body":"> sub _close_hash_and_insert_object {\n> \tmy ($self) = @_;\n>\n> \treturn unless defined($self->{hash_object_pid});\n>\n> \tmy @vars = map { 'hash_object_' . $_ } qw(pid in out ctx);\n>\n> \tcommand_close_bidi_pipe($self->{@vars});\n> \tdelete $self->{@vars};\n> }\n> $self->{@vars} evaluates to undef. i can't find any mention of using\n> arrays to dereference objects in the manual and elsewhere; is this a\n> mistake?\n\n\nThis is a hash slice notation, returning an array of hash values  \nmatching the corresponding keys.  5.10 removed some syntax warts in  \nthe case of hash slices; this is more portably expressed as @{$self} \n{@vars}; this should work in 5.10 and earlier versions, and so is the  \npreferred syntax.\n\nRegards,\n\nDavid\n--\nDavid Christensen\nEnd Point Corporation\ndavid@endpoint.com\n"},{"id":"86261","messageId":"7v7iawxa6m.fsf@gitster.siamese.dyndns.org","threadId":"14828","inReplyTo":"20080804113827.GA1239@toroid.org","subject":"Re: [PATCH] Git.pm: localise $? in command_close_bidi_pipe()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-05T06:12:01Z","receivedAt":"2008-08-05T06:12:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Abhijit Menon-Sen <ams@toroid.org> writes:\n\n> With my patch, however, Git::DESTROY calls the two _close functions\n> during global destruction, which in turn call command_close_bidi_pipe,\n> which calls waitpid with sensible arguments this time, which alters $?,\n> thus altering the exit status of the dcommit itself to 0. Oops.\n\nThanks.\n"}]}