{"thread":{"id":"48231","subject":"git 2.17.0: uninitialized value $rec in scalar chomp at ...Git.pm line 557","startedAt":"2018-04-06T12:01:09Z","lastAt":"2018-04-09T02:09:16Z","messageCount":11,"participants":["Joseph Mingrone","Johannes Schindelin","Ævar Arnfjörð Bjarmason","Eric Wong","brian m. carlson","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"343955","messageId":"86h8oobl36.fsf@phe.ftfl.ca","threadId":"48231","inReplyTo":null,"subject":"git 2.17.0: uninitialized value $rec in scalar chomp at ...Git.pm line 557","fromName":"Joseph Mingrone","fromEmail":"jrm@ftfl.ca","sentAt":"2018-04-06T12:01:01Z","receivedAt":"2018-04-06T12:01:09Z","isPatch":false,"sender":{"key":"jrm@ftfl.ca","avatar":"https://gravatar.com/avatar/6faaf635a0a3933e4bad6272de32e73fd165252515b9fb497549ca684c2bddf9?d=mp&s=160"},"body":"Hello,\n\nAfter upgrading to version 2.17.0, this message appears repeatedly when\nrunning `git svn rebase`:\n\nUse of uninitialized value $rec in scalar chomp at /usr/local/lib/perl5/site_perl/Git.pm line 557, <$fh> chunk 1.\n\nThe value of chunk varies. For example the message above may end with\n'...<$fh> chunk 5.' This is with the FreeBSD packages git-2.17.0 and\nperl 5.26.1.\n\nRegards,\n\nJ.\n"},{"id":"343958","messageId":"nycvar.QRO.7.76.6.1804061420340.55@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"48231","inReplyTo":"86h8oobl36.fsf@phe.ftfl.ca","subject":"Re: git 2.17.0: uninitialized value $rec in scalar chomp at ...Git.pm line 557","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-04-06T12:21:31Z","receivedAt":"2018-04-06T12:22:00Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Joseph,\n\nOn Fri, 6 Apr 2018, Joseph Mingrone wrote:\n\n> After upgrading to version 2.17.0, this message appears repeatedly when\n> running `git svn rebase`:\n> \n> Use of uninitialized value $rec in scalar chomp at /usr/local/lib/perl5/site_perl/Git.pm line 557, <$fh> chunk 1.\n> \n> The value of chunk varies. For example the message above may end with\n> '...<$fh> chunk 5.' This is with the FreeBSD packages git-2.17.0 and\n> perl 5.26.1.\n\nDoes this stop the `rebase`?\n\nAlso: could you publish the unrebased branch so that others can reproduce\nthe error? (I am not claiming that I have enough time; I don't...)\n\nThanks,\nJohannes\n"},{"id":"343959","messageId":"86d0zcbjn2.fsf@phe.ftfl.ca","threadId":"48231","inReplyTo":"nycvar.QRO.7.76.6.1804061420340.55@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","subject":"Re: git 2.17.0: uninitialized value $rec in scalar chomp at ...Git.pm line 557","fromName":"Joseph Mingrone","fromEmail":"jrm@ftfl.ca","sentAt":"2018-04-06T12:32:17Z","receivedAt":"2018-04-06T12:32:25Z","isPatch":false,"sender":{"key":"jrm@ftfl.ca","avatar":"https://gravatar.com/avatar/6faaf635a0a3933e4bad6272de32e73fd165252515b9fb497549ca684c2bddf9?d=mp&s=160"},"body":"Johannes Schindelin writes:\n\n> Hi Joseph,\n\n> On Fri, 6 Apr 2018, Joseph Mingrone wrote:\n\n>> After upgrading to version 2.17.0, this message appears repeatedly when\n>> running `git svn rebase`:\n\n>> Use of uninitialized value $rec in scalar chomp at /usr/local/lib/perl5/site_perl/Git.pm line 557, <$fh> chunk 1.\n\n>> The value of chunk varies. For example the message above may end with\n>> '...<$fh> chunk 5.' This is with the FreeBSD packages git-2.17.0 and\n>> perl 5.26.1.\n\n> Does this stop the `rebase`?\n\n> Also: could you publish the unrebased branch so that others can reproduce\n> the error? (I am not claiming that I have enough time; I don't...)\n\n> Thanks,\n> Johannes\n\nHi Johannes,\n\nNo, this does not stop the `svn rebase`. This is with the FreeBSD ports tree, svn.freebsd.org/ports, following the setup described at https://wiki.freebsd.org/GitWorkflow/GitSvn .\n\nJ.\n"},{"id":"343961","messageId":"20180406131514.740-1-avarab@gmail.com","threadId":"48231","inReplyTo":"86h8oobl36.fsf@phe.ftfl.ca","subject":"[PATCH] git-svn: avoid warning on undef readline()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-04-06T13:15:14Z","receivedAt":"2018-04-06T13:15:32Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change code in Git.pm that sometimes calls chomp() on undef to only do\nso the value is defined.\n\nThis code has been chomping undef values ever since it was added in\nb26098fc2f (\"git-svn: reduce scope of input record separator change\",\n2016-10-14), but started warning due to the introduction of \"use\nwarnings\" to Git.pm in my f0e19cb7ce (\"Git.pm: add the \"use warnings\"\npragma\", 2018-02-25) released with 2.17.0.\n\nSince this function will return undef in those cases it's still\npossible that the code using it will warn if it does a chomp of its\nown, as the code added in b26098fc2f (\"git-svn: reduce scope of input\nrecord separator change\", 2016-10-14) might do, but since git-svn has\n\"use warnings\" already that's clearly not a codepath that's going to\nwarn.\n\nSee https://public-inbox.org/git/86h8oobl36.fsf@phe.ftfl.ca/ for the\noriginal report.\n\nReported-by: Joseph Mingrone \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n perl/Git.pm | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex 16ebcc612c..6b6079c456 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -554,7 +554,7 @@ sub get_record {\n \tmy ($fh, $rs) = @_;\n \tlocal $/ = $rs;\n \tmy $rec = <$fh>;\n-\tchomp $rec if defined $rs;\n+\tchomp $rec if defined $rs and defined $rec;\n \t$rec;\n }\n \n-- \n2.17.0.rc1.321.gba9d0f2565\n\n"},{"id":"343962","messageId":"nycvar.QRO.7.76.6.1804061630110.55@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"48231","inReplyTo":"20180406131514.740-1-avarab@gmail.com","subject":"Re: [PATCH] git-svn: avoid warning on undef readline()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-04-06T14:30:44Z","receivedAt":"2018-04-06T14:31:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Ævar,\n\nOn Fri, 6 Apr 2018, Ævar Arnfjörð Bjarmason wrote:\n\n> Change code in Git.pm that sometimes calls chomp() on undef to only do\n> so the value is defined.\n> \n> This code has been chomping undef values ever since it was added in\n> b26098fc2f (\"git-svn: reduce scope of input record separator change\",\n> 2016-10-14), but started warning due to the introduction of \"use\n> warnings\" to Git.pm in my f0e19cb7ce (\"Git.pm: add the \"use warnings\"\n> pragma\", 2018-02-25) released with 2.17.0.\n> \n> Since this function will return undef in those cases it's still\n> possible that the code using it will warn if it does a chomp of its\n> own, as the code added in b26098fc2f (\"git-svn: reduce scope of input\n> record separator change\", 2016-10-14) might do, but since git-svn has\n> \"use warnings\" already that's clearly not a codepath that's going to\n> warn.\n> \n> See https://public-inbox.org/git/86h8oobl36.fsf@phe.ftfl.ca/ for the\n> original report.\n> \n> Reported-by: Joseph Mingrone \n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> perl/Git.pm | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/perl/Git.pm b/perl/Git.pm\n> index 16ebcc612c..6b6079c456 100644\n> --- a/perl/Git.pm\n> +++ b/perl/Git.pm\n> @@ -554,7 +554,7 @@ sub get_record {\n> \tmy ($fh, $rs) = @_;\n> \tlocal $/ = $rs;\n> \tmy $rec = <$fh>;\n> -\tchomp $rec if defined $rs;\n> +\tchomp $rec if defined $rs and defined $rec;\n> \t$rec;\n> }\n\nThe explanation as well as the patch make a total lot of sense to me.\n\nCiao,\nDscho"},{"id":"343972","messageId":"20180406165618.GA6367@80x24.org","threadId":"48231","inReplyTo":"20180406131514.740-1-avarab@gmail.com","subject":"Re: [PATCH] git-svn: avoid warning on undef readline()","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2018-04-06T16:56:18Z","receivedAt":"2018-04-06T16:56:23Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Ævar Arnfjörð Bjarmason wrote:\n> See https://public-inbox.org/git/86h8oobl36.fsf@phe.ftfl.ca/ for the\n> original report.\n\nThanks for taking a look at this. Also https://bugs.debian.org/894997\n\n> --- a/perl/Git.pm\n> +++ b/perl/Git.pm\n> @@ -554,7 +554,7 @@ sub get_record {\n> \tmy ($fh, $rs) = @_;\n> \tlocal $/ = $rs;\n> \tmy $rec = <$fh>;\n> -\tchomp $rec if defined $rs;\n> +\tchomp $rec if defined $rs and defined $rec;\n\nI'm struggling to understand the reason for the \"defined $rs\"\ncheck. I think it was a braino on my part and meant to use:\n\n\tchomp $rec if defined $rec;\n"},{"id":"343977","messageId":"87y3i0yz1t.fsf@evledraar.gmail.com","threadId":"48231","inReplyTo":"20180406165618.GA6367@80x24.org","subject":"Re: [PATCH] git-svn: avoid warning on undef readline()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-04-06T18:23:10Z","receivedAt":"2018-04-06T18:23:18Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Apr 06 2018, Eric Wong wrote:\n\n> Ævar Arnfjörð Bjarmason wrote:\n>> See https://public-inbox.org/git/86h8oobl36.fsf@phe.ftfl.ca/ for the\n>> original report.\n>\n> Thanks for taking a look at this. Also https://bugs.debian.org/894997\n>\n>> --- a/perl/Git.pm\n>> +++ b/perl/Git.pm\n>> @@ -554,7 +554,7 @@ sub get_record {\n>> \tmy ($fh, $rs) = @_;\n>> \tlocal $/ = $rs;\n>> \tmy $rec = <$fh>;\n>> -\tchomp $rec if defined $rs;\n>> +\tchomp $rec if defined $rs and defined $rec;\n>\n> I'm struggling to understand the reason for the \"defined $rs\"\n> check. I think it was a braino on my part and meant to use:\n>\n> \tchomp $rec if defined $rec;\n\nWhether this makes any sense is another question, but you seem to have\nexplicitly meant this at the time. The full function definition with\ndocumentation:\n\n =item get_record ( FILEHANDLE, INPUT_RECORD_SEPARATOR )\n\n Read one record from FILEHANDLE delimited by INPUT_RECORD_SEPARATOR,\n removing any trailing INPUT_RECORD_SEPARATOR.\n\n =cut\n\n sub get_record {\n \tmy ($fh, $rs) = @_;\n \tlocal $/ = $rs;\n \tmy $rec = <$fh>;\n \tchomp $rec if defined $rs;\n \t$rec;\n }\n\nIt doesn't make to remove the trailing record separator if it's not\ndefined, otherwise we'd be coercing undef to \"\\n\" while at the same time\nreturning multiple records. But then of course the only user of this\nwith an \"undef\" argument just does:\n\n chomp($log_entry{log} = get_record($log_fh, undef));\n\nSo we could also remove that chomp(), adn not check defined $rs, but IMO\nit's cleaner & more consistent this way.\n"},{"id":"344007","messageId":"20180406204906.GA27677@whir","threadId":"48231","inReplyTo":"87y3i0yz1t.fsf@evledraar.gmail.com","subject":"Re: [PATCH] git-svn: avoid warning on undef readline()","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2018-04-06T20:49:07Z","receivedAt":"2018-04-06T20:49:12Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Ævar Arnfjörð Bjarmason wrote:\n> On Fri, Apr 06 2018, Eric Wong wrote:\n> > Ævar Arnfjörð Bjarmason wrote:\n> >\n> >> --- a/perl/Git.pm\n> >> +++ b/perl/Git.pm\n> >> @@ -554,7 +554,7 @@ sub get_record {\n> >> \tmy ($fh, $rs) = @_;\n> >> \tlocal $/ = $rs;\n> >> \tmy $rec = <$fh>;\n> >> -\tchomp $rec if defined $rs;\n> >> +\tchomp $rec if defined $rs and defined $rec;\n> >\n> > I'm struggling to understand the reason for the \"defined $rs\"\n> > check. I think it was a braino on my part and meant to use:\n> >\n> > \tchomp $rec if defined $rec;\n> \n> Whether this makes any sense is another question, but you seem to have\n> explicitly meant this at the time. The full function definition with\n> documentation:\n> \n> =item get_record ( FILEHANDLE, INPUT_RECORD_SEPARATOR )\n> \n> Read one record from FILEHANDLE delimited by INPUT_RECORD_SEPARATOR,\n> removing any trailing INPUT_RECORD_SEPARATOR.\n\nI've always known chomp to respect the value of $/; so chomp($rec)\nwhould only cut out whatever $rs is, and be a no-op if $rs is undef.\n\n> It doesn't make to remove the trailing record separator if it's not\n> defined, otherwise we'd be coercing undef to \"\\n\" while at the same time\n> returning multiple records. But then of course the only user of this\n> with an \"undef\" argument just does:\n> \n> chomp($log_entry{log} = get_record($log_fh, undef));\n\nSubtle difference, that chomp() still sees $/ as \"\\n\".\n$/ is only undef inside get_record.\n\n> So we could also remove that chomp(), adn not check defined $rs, but IMO\n> it's cleaner & more consistent this way.\n\nI think the chomp is necessary. In git-svn.perl /^sub get_commit_entry {:\n\n\t# \n\topen my $log_fh, '>', $commit_editmsg or croak $!;\n\n\t# \n\t\t$msgbuf =~ s/\\s+$//s;\n\n\t# \n\n\t\tprint $log_fh $msgbuf or croak $!;\n\n\t# \n\tclose $log_fh or croak $!;\n\n# Above, we ensured the contents of $commit_editmsg has no trailing newline\n\n\tif ($_edit || ($type eq 'tree')) {\n\t\tchomp(my $editor = command_oneline(qw(var GIT_EDITOR)));\n\t\tsystem('sh', '-c', $editor.' \"$@\"', $editor, $commit_editmsg);\n\t}\n\n# However, $editor is likely to introduce a trailing newline\n\n\trename $commit_editmsg, $commit_msg or croak $!;\n\t{\n\t\trequire Encode;\n\t\t# SVN requires messages to be UTF-8 when entering the repo\n\t\topen $log_fh, '<', $commit_msg or croak $!;\n\t\tbinmode $log_fh;\n\n# chomp trailing newline introduced by $editor:\n\n\t\tchomp($log_entry{log} = get_record($log_fh, undef));\n"},{"id":"344008","messageId":"87sh88yrjn.fsf@evledraar.gmail.com","threadId":"48231","inReplyTo":"20180406204906.GA27677@whir","subject":"Re: [PATCH] git-svn: avoid warning on undef readline()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-04-06T21:05:16Z","receivedAt":"2018-04-06T21:05:37Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Apr 06 2018, Eric Wong wrote:\n\n> Ævar Arnfjörð Bjarmason wrote:\n>> On Fri, Apr 06 2018, Eric Wong wrote:\n>> > Ævar Arnfjörð Bjarmason wrote:\n>> >\n>> >> --- a/perl/Git.pm\n>> >> +++ b/perl/Git.pm\n>> >> @@ -554,7 +554,7 @@ sub get_record {\n>> >> \tmy ($fh, $rs) = @_;\n>> >> \tlocal $/ = $rs;\n>> >> \tmy $rec = <$fh>;\n>> >> -\tchomp $rec if defined $rs;\n>> >> +\tchomp $rec if defined $rs and defined $rec;\n>> >\n>> > I'm struggling to understand the reason for the \"defined $rs\"\n>> > check. I think it was a braino on my part and meant to use:\n>> >\n>> > \tchomp $rec if defined $rec;\n>>\n>> Whether this makes any sense is another question, but you seem to have\n>> explicitly meant this at the time. The full function definition with\n>> documentation:\n>>\n>> =item get_record ( FILEHANDLE, INPUT_RECORD_SEPARATOR )\n>>\n>> Read one record from FILEHANDLE delimited by INPUT_RECORD_SEPARATOR,\n>> removing any trailing INPUT_RECORD_SEPARATOR.\n>\n> I've always known chomp to respect the value of $/; so chomp($rec)\n> whould only cut out whatever $rs is, and be a no-op if $rs is undef.\n\nYup, you're right. I missed that.\n"},{"id":"344075","messageId":"20180407192022.GD9954@genre.crustytoothpaste.net","threadId":"48231","inReplyTo":"20180406131514.740-1-avarab@gmail.com","subject":"Re: [PATCH] git-svn: avoid warning on undef readline()","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-04-07T19:20:23Z","receivedAt":"2018-04-07T19:20:38Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Fri, Apr 06, 2018 at 01:15:14PM +0000, Ævar Arnfjörð Bjarmason wrote:\n> Change code in Git.pm that sometimes calls chomp() on undef to only do\n> so the value is defined.\n> \n> This code has been chomping undef values ever since it was added in\n> b26098fc2f (\"git-svn: reduce scope of input record separator change\",\n> 2016-10-14), but started warning due to the introduction of \"use\n> warnings\" to Git.pm in my f0e19cb7ce (\"Git.pm: add the \"use warnings\"\n> pragma\", 2018-02-25) released with 2.17.0.\n> \n> Since this function will return undef in those cases it's still\n> possible that the code using it will warn if it does a chomp of its\n> own, as the code added in b26098fc2f (\"git-svn: reduce scope of input\n> record separator change\", 2016-10-14) might do, but since git-svn has\n> \"use warnings\" already that's clearly not a codepath that's going to\n> warn.\n\nThanks for this patch. I ran into this earlier this week (with git svn\nfetch) and had intended to send a patch, but you clearly got to it\nbefore I did.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"344145","messageId":"xmqqa7ud6sho.fsf@gitster-ct.c.googlers.com","threadId":"48231","inReplyTo":"20180406165618.GA6367@80x24.org","subject":"Re: [PATCH] git-svn: avoid warning on undef readline()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-04-09T02:09:07Z","receivedAt":"2018-04-09T02:09:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong writes:\n\n> Ævar Arnfjörð Bjarmason wrote:\n>> See https://public-inbox.org/git/86h8oobl36.fsf@phe.ftfl.ca/ for the\n>> original report.\n>\n> Thanks for taking a look at this. Also https://bugs.debian.org/894997\n>\n>> --- a/perl/Git.pm\n>> +++ b/perl/Git.pm\n>> @@ -554,7 +554,7 @@ sub get_record {\n>> \tmy ($fh, $rs) = @_;\n>> \tlocal $/ = $rs;\n>> \tmy $rec = <$fh>;\n>> -\tchomp $rec if defined $rs;\n>> +\tchomp $rec if defined $rs and defined $rec;\n>\n> I'm struggling to understand the reason for the \"defined $rs\"\n> check. I think it was a braino on my part and meant to use:\n>\n> \tchomp $rec if defined $rec;\n\nThat sounds quite plausible, so if there is no further discussion,\nlet me queue a version that does s/rs/rec/ on that line myself\n(bypassing a pull from your repository at bogomips.org/).\n\nThanks.\n\n\n"}]}