{"thread":{"id":"48666","subject":"[RFC PATCH 1/2] git-credential-netrc: make \"all\" default target of Makefile","startedAt":"2018-06-07T05:20:08Z","lastAt":"2018-06-13T17:43:52Z","messageCount":20,"participants":["Todd Zullinger","Luis Marsano","Ævar Arnfjörð Bjarmason","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"349594","messageId":"20180607051958.759-2-tmz@pobox.com","threadId":"48666","inReplyTo":"20180607051958.759-1-tmz@pobox.com","subject":"[RFC PATCH 1/2] git-credential-netrc: make \"all\" default target of Makefile","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2018-06-07T05:19:57Z","receivedAt":"2018-06-07T05:20:08Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Running \"make\" in contrib/credential/netrc should run the \"all\" target\nrather than the \"test\" target. Add an empty \"all::\" target like most of\nour other Makefiles.\n\nSigned-off-by: Todd Zullinger \n---\n contrib/credential/netrc/Makefile | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/contrib/credential/netrc/Makefile b/contrib/credential/netrc/Makefile\nindex 0ffa407191..6174e3bb83 100644\n--- a/contrib/credential/netrc/Makefile\n+++ b/contrib/credential/netrc/Makefile\n@@ -1,3 +1,6 @@\n+# The default target of this Makefile is...\n+all::\n+\n test:\n \t./t-git-credential-netrc.sh\n \n-- \n2.18.0.rc1\n\n"},{"id":"349595","messageId":"20180607051958.759-1-tmz@pobox.com","threadId":"48666","inReplyTo":null,"subject":"[RFC PATCH 0/2] contrib/credential/netrc Makefile & test improvements","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2018-06-07T05:19:56Z","receivedAt":"2018-06-07T05:20:11Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Hi,\n\nI noticed failures from the contrib/credential/netrc tests\nwhile building 2.18.0 release candidates. I was surprised\nto see the tests being run when called with a simple 'make'\ncommand.\n\nThe first patch in the series adds an empty 'all::' make\ntarget to match most of our other Makefiles and avoid the\nsurprise of running tests by default. (When the netrc\nhelper was added to the fedora builds, it copied the same\n'make -C contrib/credential/...' pattern from other\ncredential helpers -- despite the lack of anything to\nbuild.)\n\nThe actual test failures were initially due to my build\nenvironment lacking the perl autodie module, which was added\nin 786ef50a23 (\"git-credential-netrc: accept gpg option\",\n2018-05-12).\n\nAfter installing the autodie module, the failures were due\nto the build environment lacking a git install (specifically\nthe perl Git module). The tests needing a pre-installed\nperl Git seemed odd and worth fixing.\n\nThe second patch in the series aims to fix this. I'm not\nsure if there's a better or more preferable way to fix this,\nwhich is one of the reasons for the RFC tag. (It's also why\nI added you to the Cc Ævar, as you're one of the\nknowledgeable perl folks here.)\n\nThe other reason for the RFC tag is that I'm unsure of how\nto fix the last issue I found. The tests exit cleanly even\nwhen there are failures, which seems undesirable. I'm not\nfamiliar with the perl test_external framework to suggest a\nfix in patch form. It might be a matter of adding something\nlike this, from t/t9700/test.pl:\n\n my $is_passing = eval { Test::More->is_passing };\n exit($is_passing ? 0 : 1) unless $@ =~ /Can't locate object method/;\n\n? But that's a wild guess which I haven't tested.\n\nHere's the output from 'make test' showing that most tests\nfail and we still get a clean exit status:\n\n$ make -C contrib/credential/netrc test ; echo \"netrc test exit status: $?\"\nmake: Entering directory '/builddir/build/BUILD/git-2.18.0.rc1/contrib/credential/netrc'\n./t-git-credential-netrc.sh\nok 1 - set up test repository\n# run 1: git-credential-netrc (perl /builddir/build/BUILD/git-2.18.0.rc1/t/../contrib/credential/netrc/test.pl)\nok 2 - Got 0 keys from insecure file\nok 3 - Got 0 keys from missing file\nnot ok 4 - Got first found keys with bad data\nok 5 - Got no corovamilkbar keys\nnot ok 6 - Got 2 Github keys\nnot ok 7 - Got correct Github password\nnot ok 8 - Got correct Github username\nnot ok 9 - Got 2 username-specific keys\nnot ok 10 - Got correct user-specific password\nnot ok 11 - Got correct user-specific protocol\nnot ok 12 - Got 2 host:port-specific keys\nnot ok 13 - Got correct host:port-specific password\nnot ok 14 - Got correct host:port-specific username\nnot ok 15 - Got 2 'host:port kills host' keys\nnot ok 16 - Got correct 'host:port kills host' password\nnot ok 17 - Got correct 'host:port kills host' username\nnot ok 18 - Got keys decrypted by git config option\nnot ok 19 - Got keys decrypted by command option\n# test_external test git-credential-netrc was ok\nmake: Leaving directory '/builddir/build/BUILD/git-2.18.0.rc1/contrib/credential/netrc'\nnetrc test exit status: 0\n\nTodd Zullinger (2):\n git-credential-netrc: make \"all\" default target of Makefile\n git-credential-netrc: use in-tree Git.pm for tests\n\n contrib/credential/netrc/Makefile | 3 +++\n contrib/credential/netrc/test.pl | 3 +++\n 2 files changed, 6 insertions(+)\n\nThanks all.\n\n-- \n2.18.0.rc1\n"},{"id":"349596","messageId":"20180607051958.759-3-tmz@pobox.com","threadId":"48666","inReplyTo":"20180607051958.759-1-tmz@pobox.com","subject":"[RFC PATCH 2/2] git-credential-netrc: use in-tree Git.pm for tests","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2018-06-07T05:19:58Z","receivedAt":"2018-06-07T05:20:12Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"The netrc test.pl script calls git-credential-netrc which imports the\nGit module. Pass GITPERLLIB to git-credential-netrc via PERL5LIB to\nensure the in-tree Git module is used for testing.\n\nSigned-off-by: Todd Zullinger \n---\n contrib/credential/netrc/test.pl | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/contrib/credential/netrc/test.pl b/contrib/credential/netrc/test.pl\nindex 1e1001030e..5e26b4d190 100755\n--- a/contrib/credential/netrc/test.pl\n+++ b/contrib/credential/netrc/test.pl\n@@ -27,6 +27,9 @@ BEGIN\n ? $ENV{PATH}\n : ();\n \n+# use in-tree Git.pm\n+local $ENV{PERL5LIB} = $ENV{GITPERLLIB};\n+\n diag \"Testing insecure file, nothing should be found\\n\";\n chmod 0644, $netrc;\n my $cred = run_credential(['-f', $netrc, 'get'],\n-- \n2.18.0.rc1\n\n"},{"id":"349830","messageId":"CAHqJXRHGq9ofPcCzffUbTmuL6fuoB=33aZnwqNdqYy+nAVA_gw@mail.gmail.com","threadId":"48666","inReplyTo":"20180607051958.759-1-tmz@pobox.com","subject":"Re: [RFC PATCH 0/2] contrib/credential/netrc Makefile & test improvements","fromName":"Luis Marsano","fromEmail":"luis.marsano@gmail.com","sentAt":"2018-06-09T21:18:04Z","receivedAt":"2018-06-09T21:18:20Z","isPatch":true,"sender":{"key":"luis.marsano@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8139324?v=4"},"body":"Thanks for looking into this and addressing these issues.\n\nOn Thu, Jun 7, 2018 at 1:20 AM Todd Zullinger wrote:\n>\n> Hi,\n>\n> I noticed failures from the contrib/credential/netrc tests\n> while building 2.18.0 release candidates. I was surprised\n> to see the tests being run when called with a simple 'make'\n> command.\n>\n> The first patch in the series adds an empty 'all::' make\n> target to match most of our other Makefiles and avoid the\n> surprise of running tests by default. (When the netrc\n> helper was added to the fedora builds, it copied the same\n> 'make -C contrib/credential/...' pattern from other\n> credential helpers -- despite the lack of anything to\n> build.)\n\nI think this is a good idea.\n\n> The actual test failures were initially due to my build\n> environment lacking the perl autodie module, which was added\n> in 786ef50a23 (\"git-credential-netrc: accept gpg option\",\n> 2018-05-12).\n\nI added 'use autodie;' without realizing it had external dependencies.\nAccording to the documentation\nhttp://perldoc.perl.org/autodie.html\nit's a pragma since perl 5.10.1\nRemoving 'use autodie;' should be fine: it's not critical.\n\n> After installing the autodie module, the failures were due\n> to the build environment lacking a git install (specifically\n> the perl Git module). The tests needing a pre-installed\n> perl Git seemed odd and worth fixing.\n\nI mistakenly thought 'use lib (split(/:/, $ENV{GITPERLLIB}));' in\ntest.pl handled this.\nSince it doesn't, and I was only following an example from\nt/t9700/test.pl that doesn't fit, this line should be removed and it\nmight make more sense to set the environment from\nt-git-credential-netrc.sh near '. ./test-lib.sh', which also sets the\nenvironment.\nSomething like\n\ndiff --git a/contrib/credential/netrc/t-git-credential-netrc.sh\nb/contrib/credential/netrc/t-git-credential-netrc.sh\nindex 58191a6..9e18611 100755\n--- a/contrib/credential/netrc/t-git-credential-netrc.sh\n+++ b/contrib/credential/netrc/t-git-credential-netrc.sh\n@@ -23,5 +23,6 @@\n # The external test will outputs its own plan\n test_external_has_tap=1\n\n+ export PERL5LIB=\"$GITPERLLIB\"\n test_external \\\n 'git-credential-netrc' \\\n\nYour solution, however, is reasonable, and I don't know which is preferred.\n\n> The second patch in the series aims to fix this. I'm not\n> sure if there's a better or more preferable way to fix this,\n> which is one of the reasons for the RFC tag. (It's also why\n> I added you to the Cc Ævar, as you're one of the\n> knowledgeable perl folks here.)\n>\n> The other reason for the RFC tag is that I'm unsure of how\n> to fix the last issue I found. The tests exit cleanly even\n> when there are failures, which seems undesirable. I'm not\n> familiar with the perl test_external framework to suggest a\n> fix in patch form. It might be a matter of adding something\n> like this, from t/t9700/test.pl:\n>\n> my $is_passing = eval { Test::More->is_passing };\n> exit($is_passing ? 0 : 1) unless $@ =~ /Can't locate object method/;\n>\n> ? But that's a wild guess which I haven't tested.\n\nGood idea.\nIt looks like you found an issue with t/t9700/test.pl, too.\nWhen altered to fail, it first reports ok (then reports failed and\nreturns non-0).\n\nnot ok 46 - unquote simple quoted path\nnot ok 47 - unquote escape sequences\n1..47\n# test_external test Perl API was ok\n# test_external_without_stderr test no stderr: Perl API failed: perl\n/home/luism/project/git/t/t9700/test.pl:\n$ echo $?\n1\n\nTo make git-credential-netrc tests behave correctly, I ended up making\nthe changes below.\nThey might be okay, unless someone knows better.\n\ndiff --git a/contrib/credential/netrc/t-git-credential-netrc.sh\nb/contrib/credential/netrc/t-git-credential-netrc.sh\nindex 58191a6..9e18611 100755\n--- a/contrib/credential/netrc/t-git-credential-netrc.sh\n+++ b/contrib/credential/netrc/t-git-credential-netrc.sh\n@@ -23,9 +23,10 @@\n # The external test will outputs its own plan\n test_external_has_tap=1\n\n+ export PERL5LIB=\"$GITPERLLIB\"\n test_external \\\n 'git-credential-netrc' \\\n- perl \"$TEST_DIRECTORY\"/../contrib/credential/netrc/test.pl\n+ perl \"$GIT_BUILD_DIR\"/contrib/credential/netrc/test.pl\n\n test_done\n )\ndiff --git a/contrib/credential/netrc/test.pl b/contrib/credential/netrc/test.pl\nindex 1e10010..abc9081 100755\n--- a/contrib/credential/netrc/test.pl\n+++ b/contrib/credential/netrc/test.pl\n@@ -1,6 +1,4 @@\n #!/usr/bin/perl\n-use lib (split(/:/, $ENV{GITPERLLIB}));\n-\n use warnings;\n use strict;\n use Test::More qw(no_plan);\n@@ -12,7 +10,6 @@ BEGIN\n # t-git-credential-netrc.sh kicks off our testing, so we have to go\n # from there.\n Test::More->builder->current_test(1);\n- Test::More->builder->no_ending(1);\n }\n\n my @global_credential_args = @ARGV;\n@@ -104,6 +101,9 @@ BEGIN\n\n ok(scalar keys %$cred == 2, 'Got keys decrypted by command option');\n\n+my $is_passing = eval { Test::More->is_passing };\n+exit($is_passing ? 0 : 1) unless $@ =~ /Can't locate object method/;\n+\n sub run_credential\n {\n my $args = shift @_;\n"},{"id":"349845","messageId":"20180610022802.GX3094@zaya.teonanacatl.net","threadId":"48666","inReplyTo":"CAHqJXRHGq9ofPcCzffUbTmuL6fuoB=33aZnwqNdqYy+nAVA_gw@mail.gmail.com","subject":"Re: [RFC PATCH 0/2] contrib/credential/netrc Makefile & test improvements","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2018-06-10T02:28:02Z","receivedAt":"2018-06-10T02:28:09Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Hi Luis,\n\nLuis Marsano wrote:\n> Thanks for looking into this and addressing these issues.\n\nAnd thank you for digging further. :)\n\n> On Thu, Jun 7, 2018 at 1:20 AM Todd Zullinger wrote:\n>> I noticed failures from the contrib/credential/netrc tests\n>> while building 2.18.0 release candidates. I was surprised\n>> to see the tests being run when called with a simple 'make'\n>> command.\n>>\n>> The first patch in the series adds an empty 'all::' make\n>> target to match most of our other Makefiles and avoid the\n>> surprise of running tests by default. (When the netrc\n>> helper was added to the fedora builds, it copied the same\n>> 'make -C contrib/credential/...' pattern from other\n>> credential helpers -- despite the lack of anything to\n>> build.)\n> \n> I think this is a good idea.\n> \n>> The actual test failures were initially due to my build\n>> environment lacking the perl autodie module, which was added\n>> in 786ef50a23 (\"git-credential-netrc: accept gpg option\",\n>> 2018-05-12).\n> \n> I added 'use autodie;' without realizing it had external dependencies.\n> According to the documentation\n> http://perldoc.perl.org/autodie.html\n> it's a pragma since perl 5.10.1\n> Removing 'use autodie;' should be fine: it's not critical.\n\nI should clarify that part of why autodie isn't in my build\nenvironment is that the Fedora and RHEL7+ perl packages\nsplit out many modules which are shipped as part of the core\nperl tarball. So while all the platforms I care about have\nperl >= 5.10.1, the Fedora and newer RHEL systems have the\nautodie module in a separate package.\n\nThat said, the INSTALL docs still suggest that we only\nrequire perl >= 5.8, so if autodie is easily removed, that\nwould probably be a good plan.\n\nÆvar brought up bumping the minimum supported perl to 5.10.0\nlast December, in <20171223174400.26668-1-avarab@gmail.com>\n(https://public-inbox.org/git/20171223174400.26668-1-avarab@gmail.com/).\nThe general consensus seemed positive, but I don't think\nit's happened. Even so, that was 5.10.0, not the 5.10.1\nwhich added autodie.\n\n>> After installing the autodie module, the failures were due\n>> to the build environment lacking a git install (specifically\n>> the perl Git module). The tests needing a pre-installed\n>> perl Git seemed odd and worth fixing.\n> \n> I mistakenly thought 'use lib (split(/:/, $ENV{GITPERLLIB}));' in\n> test.pl handled this.\n> Since it doesn't, and I was only following an example from\n> t/t9700/test.pl that doesn't fit, this line should be removed and it\n> might make more sense to set the environment from\n> t-git-credential-netrc.sh near '. ./test-lib.sh', which also sets the\n> environment.\n> Something like\n> \n> diff --git a/contrib/credential/netrc/t-git-credential-netrc.sh\n> b/contrib/credential/netrc/t-git-credential-netrc.sh\n> index 58191a6..9e18611 100755\n> --- a/contrib/credential/netrc/t-git-credential-netrc.sh\n> +++ b/contrib/credential/netrc/t-git-credential-netrc.sh\n> @@ -23,5 +23,6 @@\n> # The external test will outputs its own plan\n> test_external_has_tap=1\n> \n> + export PERL5LIB=\"$GITPERLLIB\"\n> test_external \\\n> 'git-credential-netrc' \\\n> \n> Your solution, however, is reasonable, and I don't know which is preferred.\n\nI think your placement is better. As you say, it could also\nbe placed closer to '. ./test-lib.sh'.\n\nIt doesn't come up very often, but I wonder if there's any\ndownside to having test-lib.sh export PERL5LIB?\n\n> It looks like you found an issue with t/t9700/test.pl, too.\n> When altered to fail, it first reports ok (then reports failed and\n> returns non-0).\n> \n> not ok 46 - unquote simple quoted path\n> not ok 47 - unquote escape sequences\n> 1..47\n> # test_external test Perl API was ok\n> # test_external_without_stderr test no stderr: Perl API failed: perl\n> /home/luism/project/git/t/t9700/test.pl:\n> $ echo $?\n> 1\n\nOops. Nice catch. At least that does exit non-zero I\nguess.\n\n> To make git-credential-netrc tests behave correctly, I ended up making\n> the changes below.\n> They might be okay, unless someone knows better.\n> \n> diff --git a/contrib/credential/netrc/t-git-credential-netrc.sh\n> b/contrib/credential/netrc/t-git-credential-netrc.sh\n> index 58191a6..9e18611 100755\n> --- a/contrib/credential/netrc/t-git-credential-netrc.sh\n> +++ b/contrib/credential/netrc/t-git-credential-netrc.sh\n> @@ -23,9 +23,10 @@\n> # The external test will outputs its own plan\n> test_external_has_tap=1\n> \n> + export PERL5LIB=\"$GITPERLLIB\"\n> test_external \\\n> 'git-credential-netrc' \\\n> - perl \"$TEST_DIRECTORY\"/../contrib/credential/netrc/test.pl\n> + perl \"$GIT_BUILD_DIR\"/contrib/credential/netrc/test.pl\n> \n> test_done\n> )\n\nThis reminds me, while we're here it might be worth adding a\nwhitespace cleanup commit to indent the lines following\ntest_expect_success and test_external with 2 tabs.\n\n> diff --git a/contrib/credential/netrc/test.pl b/contrib/credential/netrc/test.pl\n> index 1e10010..abc9081 100755\n> --- a/contrib/credential/netrc/test.pl\n> +++ b/contrib/credential/netrc/test.pl\n> @@ -1,6 +1,4 @@\n> #!/usr/bin/perl\n> -use lib (split(/:/, $ENV{GITPERLLIB}));\n> -\n> use warnings;\n> use strict;\n> use Test::More qw(no_plan);\n> @@ -12,7 +10,6 @@ BEGIN\n> # t-git-credential-netrc.sh kicks off our testing, so we have to go\n> # from there.\n> Test::More->builder->current_test(1);\n> - Test::More->builder->no_ending(1);\n> }\n> \n> my @global_credential_args = @ARGV;\n> @@ -104,6 +101,9 @@ BEGIN\n> \n> ok(scalar keys %$cred == 2, 'Got keys decrypted by command option');\n> \n> +my $is_passing = eval { Test::More->is_passing };\n> +exit($is_passing ? 0 : 1) unless $@ =~ /Can't locate object method/;\n> +\n> sub run_credential\n> {\n> my $args = shift @_;\n\nI tested this and it worked well for my builds with and\nwithout the autodie module (passing and failing\nappropriately).\n\nThanks,\n\n-- \nTodd\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\nA common mistake people make when trying to design something\ncompletely foolproof is to underestimate the ingenuity of complete\nfools.\n -- Douglas Adams\n\n"},{"id":"349853","messageId":"87efhfvxzu.fsf@evledraar.gmail.com","threadId":"48666","inReplyTo":"20180610022802.GX3094@zaya.teonanacatl.net","subject":"Re: [RFC PATCH 0/2] contrib/credential/netrc Makefile & test improvements","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-06-10T08:36:53Z","receivedAt":"2018-06-10T08:37:00Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sun, Jun 10 2018, Todd Zullinger wrote:\n\n>> I added 'use autodie;' without realizing it had external dependencies.\n>> According to the documentation\n>> http://perldoc.perl.org/autodie.html\n>> it's a pragma since perl 5.10.1\n>> Removing 'use autodie;' should be fine: it's not critical.\n>\n> I should clarify that part of why autodie isn't in my build\n> environment is that the Fedora and RHEL7+ perl packages\n> split out many modules which are shipped as part of the core\n> perl tarball. So while all the platforms I care about have\n> perl >= 5.10.1, the Fedora and newer RHEL systems have the\n> autodie module in a separate package.\n>\n> That said, the INSTALL docs still suggest that we only\n> require perl >= 5.8, so if autodie is easily removed, that\n> would probably be a good plan.\n\nThe intent of those docs was and still is to say \"5.8 and the modules it\nships with\".\n\nThis was discussed when 2.17.0 was released with my changes to make us\nunconditionally use Digest::MD5:\nhttps://public-inbox.org/git/87fu50e0ht.fsf@evledraar.gmail.com/\n\nAs noted there that's not some dogmatic \"RedHat altered perl so we don't\ncare about them\", but rather that in practice this doesn't impact users\nnegatively, so I think it's fine.\n\n> Ævar brought up bumping the minimum supported perl to 5.10.0\n> last December, in <20171223174400.26668-1-avarab@gmail.com>\n> (https://public-inbox.org/git/20171223174400.26668-1-avarab@gmail.com/).\n> The general consensus seemed positive, but I don't think\n> it's happened. Even so, that was 5.10.0, not the 5.10.1\n> which added autodie.\n\nRight, this doesn't apply to autodie. Looking at\nhttps://www.cpan.org/ports/binaries.html there were a lot of releases\nthat had 5.10.0, not *.1.\n\nAlso git-credential-netrc is in contrib, I don't know if that warrants\ntreating it differently, I don't use it myself, and don't know how much\nit's \"not really contrib\" in practice (e.g. like the bash completion\nwhich is installed everywhere...)>\n\nBut yeah, skimming the code it would be easy to remove the dependency.\n"},{"id":"350039","messageId":"20180613030852.GG3094@zaya.teonanacatl.net","threadId":"48666","inReplyTo":"87efhfvxzu.fsf@evledraar.gmail.com","subject":"Re: [RFC PATCH 0/2] contrib/credential/netrc Makefile & test improvements","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2018-06-13T03:08:52Z","receivedAt":"2018-06-13T03:09:00Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Hi,\n\nÆvar Arnfjörð Bjarmason wrote:\n> \n> On Sun, Jun 10 2018, Todd Zullinger wrote:\n> \n>>> I added 'use autodie;' without realizing it had external dependencies.\n>>> According to the documentation\n>>> http://perldoc.perl.org/autodie.html\n>>> it's a pragma since perl 5.10.1\n>>> Removing 'use autodie;' should be fine: it's not critical.\n>>\n>> I should clarify that part of why autodie isn't in my build\n>> environment is that the Fedora and RHEL7+ perl packages\n>> split out many modules which are shipped as part of the core\n>> perl tarball. So while all the platforms I care about have\n>> perl >= 5.10.1, the Fedora and newer RHEL systems have the\n>> autodie module in a separate package.\n>>\n>> That said, the INSTALL docs still suggest that we only\n>> require perl >= 5.8, so if autodie is easily removed, that\n>> would probably be a good plan.\n> \n> The intent of those docs was and still is to say \"5.8 and the modules it\n> ships with\".\n> \n> This was discussed when 2.17.0 was released with my changes to make us\n> unconditionally use Digest::MD5:\n> https://public-inbox.org/git/87fu50e0ht.fsf@evledraar.gmail.com/\n> \n> As noted there that's not some dogmatic \"RedHat altered perl so we don't\n> care about them\", but rather that in practice this doesn't impact users\n> negatively, so I think it's fine.\n\nYeah, that was my understanding. I only included the\ninformation on why it was missing from my build environment\ndespite having perl-5.10.1 was due to the Fedora/Red Hat\npackaging. I agree that any issues which fall out of those\npackaging differences are on Fedora/Red Hat packagers to\nfix.\n\n(Which should all be relatively trivial, as the perl modules\ncontain a 'Provides: perl($module)'. That allows dnf|yum\ninstall 'perl(autodie)' to easily pull in the right package.\nAnd the rpm perl dep generator creates a 'Requires:\nperl(autodie)' when is sees 'use autodie'.)\n\nSorry if I muddied the conversation with that tangential\ninfo. :)\n\n>> Ævar brought up bumping the minimum supported perl to 5.10.0\n>> last December, in <20171223174400.26668-1-avarab@gmail.com>\n>> (https://public-inbox.org/git/20171223174400.26668-1-avarab@gmail.com/).\n>> The general consensus seemed positive, but I don't think\n>> it's happened. Even so, that was 5.10.0, not the 5.10.1\n>> which added autodie.\n> \n> Right, this doesn't apply to autodie. Looking at\n> https://www.cpan.org/ports/binaries.html there were a lot of releases\n> that had 5.10.0, not *.1.\n> \n> Also git-credential-netrc is in contrib, I don't know if that warrants\n> treating it differently, I don't use it myself, and don't know how much\n> it's \"not really contrib\" in practice (e.g. like the bash completion\n> which is installed everywhere...)>\n\nIndeed, that's a fine question. All the platforms I care\nabout have 5.10.1 or newer, so either way works for me.\n\n> But yeah, skimming the code it would be easy to remove the dependency.\n\nI wrapped up the changes from Luis which replace my 2/2 \"use\nin-tree Git.pm for tests\" and to ensure the tests exit\nproperly on failures. I'll send those out now in the hope\nthat it saves a little effort in moving these minor fixes\nforward.\n\nAs far as removing the autodie dep, is there anything more\nto that than dropping the 'use autodie' line? It looks like\ndoing so leaves us no worse than we were before, but I\nhaven't written any perl in a long time.\n\nThanks,\n\n-- \nTodd\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\nOcean, n. A body of water occupying about two-thirds of a world made\nfor man -- who has no gills.\n -- Ambrose Bierce, \"The Devil's Dictionary\"\n\n"},{"id":"350040","messageId":"20180613031040.3109-1-tmz@pobox.com","threadId":"48666","inReplyTo":"20180607051958.759-1-tmz@pobox.com","subject":"[RFC PATCH v2 0/4] contrib/credential/netrc Makefile & test improvements","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2018-06-13T03:10:35Z","receivedAt":"2018-06-13T03:10:46Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"This replaces my 2/2 \"use in-tree Git.pm for tests\" with\nLuis's improved version. It also adds Luis's fix to ensure\nthe proper exit status on test failures and a minor\nwhitespace cleanup.\n\nIs it alright to forge your signoff Luis?\n\nLuis Marsano (2):\n git-credential-netrc: use in-tree Git.pm for tests\n git-credential-netrc: fix exit status when tests fail\n\nTodd Zullinger (2):\n git-credential-netrc: make \"all\" default target of Makefile\n git-credential-netrc: minor whitespace cleanup in test script\n\n contrib/credential/netrc/Makefile | 3 +++\n contrib/credential/netrc/t-git-credential-netrc.sh | 9 +++++----\n contrib/credential/netrc/test.pl | 5 +++--\n 3 files changed, 11 insertions(+), 6 deletions(-)\n"},{"id":"350041","messageId":"20180613031040.3109-2-tmz@pobox.com","threadId":"48666","inReplyTo":"20180607051958.759-1-tmz@pobox.com","subject":"[RFC PATCH v2 1/4] git-credential-netrc: make \"all\" default target of Makefile","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2018-06-13T03:10:36Z","receivedAt":"2018-06-13T03:10:49Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Running \"make\" in contrib/credential/netrc should run the \"all\" target\nrather than the \"test\" target. Add an empty \"all::\" target like most of\nour other Makefiles.\n\nSigned-off-by: Todd Zullinger \n---\n contrib/credential/netrc/Makefile | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/contrib/credential/netrc/Makefile b/contrib/credential/netrc/Makefile\nindex 0ffa407191..6174e3bb83 100644\n--- a/contrib/credential/netrc/Makefile\n+++ b/contrib/credential/netrc/Makefile\n@@ -1,3 +1,6 @@\n+# The default target of this Makefile is...\n+all::\n+\n test:\n \t./t-git-credential-netrc.sh\n \n"},{"id":"350042","messageId":"20180613031040.3109-3-tmz@pobox.com","threadId":"48666","inReplyTo":"20180607051958.759-1-tmz@pobox.com","subject":"[RFC PATCH v2 2/4] git-credential-netrc: minor whitespace cleanup in test script","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2018-06-13T03:10:37Z","receivedAt":"2018-06-13T03:10:51Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Signed-off-by: Todd Zullinger \n---\n contrib/credential/netrc/t-git-credential-netrc.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/contrib/credential/netrc/t-git-credential-netrc.sh b/contrib/credential/netrc/t-git-credential-netrc.sh\nindex 58191a62f8..c5661087fe 100755\n--- a/contrib/credential/netrc/t-git-credential-netrc.sh\n+++ b/contrib/credential/netrc/t-git-credential-netrc.sh\n@@ -17,15 +17,15 @@\n \t# set up test repository\n \n \ttest_expect_success \\\n- 'set up test repository' \\\n- 'git config --add gpg.program test.git-config-gpg'\n+\t\t'set up test repository' \\\n+\t\t'git config --add gpg.program test.git-config-gpg'\n \n \t# The external test will outputs its own plan\n \ttest_external_has_tap=1\n \n \ttest_external \\\n- 'git-credential-netrc' \\\n- perl \"$TEST_DIRECTORY\"/../contrib/credential/netrc/test.pl\n+\t\t'git-credential-netrc' \\\n+\t\tperl \"$TEST_DIRECTORY\"/../contrib/credential/netrc/test.pl\n \n \ttest_done\n )\n"},{"id":"350043","messageId":"20180613031040.3109-4-tmz@pobox.com","threadId":"48666","inReplyTo":"20180607051958.759-1-tmz@pobox.com","subject":"[RFC PATCH v2 3/4] git-credential-netrc: use in-tree Git.pm for tests","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2018-06-13T03:10:38Z","receivedAt":"2018-06-13T03:10:54Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"From: Luis Marsano \n\nThe netrc test.pl script calls git-credential-netrc which imports the\nGit module. Pass GITPERLLIB to git-credential-netrc via PERL5LIB to\nensure the in-tree Git module is used for testing.\n\nSigned-off-by: Luis Marsano \n---\n contrib/credential/netrc/t-git-credential-netrc.sh | 3 ++-\n contrib/credential/netrc/test.pl | 1 -\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/credential/netrc/t-git-credential-netrc.sh b/contrib/credential/netrc/t-git-credential-netrc.sh\nindex c5661087fe..07227d0228 100755\n--- a/contrib/credential/netrc/t-git-credential-netrc.sh\n+++ b/contrib/credential/netrc/t-git-credential-netrc.sh\n@@ -23,9 +23,10 @@\n \t# The external test will outputs its own plan\n \ttest_external_has_tap=1\n \n+\texport PERL5LIB=\"$GITPERLLIB\"\n \ttest_external \\\n \t\t'git-credential-netrc' \\\n-\t\tperl \"$TEST_DIRECTORY\"/../contrib/credential/netrc/test.pl\n+\t\tperl \"$GIT_BUILD_DIR\"/contrib/credential/netrc/test.pl\n \n \ttest_done\n )\ndiff --git a/contrib/credential/netrc/test.pl b/contrib/credential/netrc/test.pl\nindex 1e1001030e..2b5280ad6a 100755\n--- a/contrib/credential/netrc/test.pl\n+++ b/contrib/credential/netrc/test.pl\n@@ -1,5 +1,4 @@\n #!/usr/bin/perl\n-use lib (split(/:/, $ENV{GITPERLLIB}));\n \n use warnings;\n use strict;\n"},{"id":"350044","messageId":"20180613031040.3109-5-tmz@pobox.com","threadId":"48666","inReplyTo":"20180607051958.759-1-tmz@pobox.com","subject":"[RFC PATCH v2 4/4] git-credential-netrc: fix exit status when tests fail","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2018-06-13T03:10:39Z","receivedAt":"2018-06-13T03:10:56Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"From: Luis Marsano \n\nSigned-off-by: Luis Marsano \n---\n contrib/credential/netrc/test.pl | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/contrib/credential/netrc/test.pl b/contrib/credential/netrc/test.pl\nindex 2b5280ad6a..c0fb3718b2 100755\n--- a/contrib/credential/netrc/test.pl\n+++ b/contrib/credential/netrc/test.pl\n@@ -11,7 +11,6 @@ BEGIN\n \t# t-git-credential-netrc.sh kicks off our testing, so we have to go\n \t# from there.\n \tTest::More->builder->current_test(1);\n-\tTest::More->builder->no_ending(1);\n }\n \n my @global_credential_args = @ARGV;\n@@ -103,6 +102,9 @@ BEGIN\n \n ok(scalar keys %$cred == 2, 'Got keys decrypted by command option');\n \n+my $is_passing = eval { Test::More->is_passing };\n+exit($is_passing ? 0 : 1) unless $@ =~ /Can't locate object method/;\n+\n sub run_credential\n {\n \tmy $args = shift @_;\n"},{"id":"350046","messageId":"CAPig+cRcPD9vh1WMX7UkiD9R_A-DrybvOYwi3R8U2bskfOV_1w@mail.gmail.com","threadId":"48666","inReplyTo":"20180613031040.3109-3-tmz@pobox.com","subject":"Re: [RFC PATCH v2 2/4] git-credential-netrc: minor whitespace cleanup in test script","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-06-13T05:42:59Z","receivedAt":"2018-06-13T05:43:04Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Jun 12, 2018 at 11:10 PM, Todd Zullinger wrote:\n> Signed-off-by: Todd Zullinger \n> ---\n> diff --git a/contrib/credential/netrc/t-git-credential-netrc.sh b/contrib/credential/netrc/t-git-credential-netrc.sh\n> index 58191a62f8..c5661087fe 100755\n> --- a/contrib/credential/netrc/t-git-credential-netrc.sh\n> +++ b/contrib/credential/netrc/t-git-credential-netrc.sh\n> @@ -17,15 +17,15 @@\n> # set up test repository\n>\n> test_expect_success \\\n> - 'set up test repository' \\\n> - 'git config --add gpg.program test.git-config-gpg'\n> + 'set up test repository' \\\n> + 'git config --add gpg.program test.git-config-gpg'\n\nSince you're touching all the tests in this script anyhow, perhaps\nmodernize them so the title and opening quote of the test body are on\nthe same line as test_expect_success, and the closing body quote is on\na line of its own?\n\n test_expect_sucess 'setup test repository' '\n ...test body...\n '\n\nI also changed \"set up\" to \"setup\" to follow existing practice.\n\n(Not necessarily worth a re-roll.)\n\n> # The external test will outputs its own plan\n> test_external_has_tap=1\n>\n> test_external \\\n> - 'git-credential-netrc' \\\n> - perl \"$TEST_DIRECTORY\"/../contrib/credential/netrc/test.pl\n> + 'git-credential-netrc' \\\n> + perl \"$TEST_DIRECTORY\"/../contrib/credential/netrc/test.pl\n>\n> test_done\n> )\n"},{"id":"350047","messageId":"CAHqJXRE8OKSKcck1APHAHccLZhox+tZi8nNu2RA74RErX8s3Pg@mail.gmail.com","threadId":"48666","inReplyTo":"20180613030852.GG3094@zaya.teonanacatl.net","subject":"Re: [RFC PATCH 0/2] contrib/credential/netrc Makefile & test improvements","fromName":"Luis Marsano","fromEmail":"luis.marsano@gmail.com","sentAt":"2018-06-13T06:15:20Z","receivedAt":"2018-06-13T06:15:37Z","isPatch":true,"sender":{"key":"luis.marsano@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8139324?v=4"},"body":"On Tue, Jun 12, 2018 at 11:08 PM Todd Zullinger wrote:\n> As far as removing the autodie dep, is there anything more\n> to that than dropping the 'use autodie' line? It looks like\n> doing so leaves us no worse than we were before, but I\n> haven't written any perl in a long time.\n\nErasing that line should remove the dependency.\nI added it as a gratuitous safeguard.\nI think it's fine to simply remove it.\nIf anyone knows better, of course, I hope they'll say.\n\n> Thanks,\n>\n> --\n> Todd\n> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n> Ocean, n. A body of water occupying about two-thirds of a world made\n> for man -- who has no gills.\n> -- Ambrose Bierce, \"The Devil's Dictionary\"\n>\n"},{"id":"350048","messageId":"CAHqJXRHZSGQF9cR8f4N5j8oSRCAoNzXnWLAsGO4witjmHU63ZA@mail.gmail.com","threadId":"48666","inReplyTo":"20180613031040.3109-1-tmz@pobox.com","subject":"Re: [RFC PATCH v2 0/4] contrib/credential/netrc Makefile & test improvements","fromName":"Luis Marsano","fromEmail":"luis.marsano@gmail.com","sentAt":"2018-06-13T06:59:17Z","receivedAt":"2018-06-13T06:59:34Z","isPatch":true,"sender":{"key":"luis.marsano@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8139324?v=4"},"body":"On Tue, Jun 12, 2018 at 11:10 PM Todd Zullinger wrote:\n>\n> This replaces my 2/2 \"use in-tree Git.pm for tests\" with\n> Luis's improved version. It also adds Luis's fix to ensure\n> the proper exit status on test failures and a minor\n> whitespace cleanup.\n>\n> Is it alright to forge your signoff Luis?\n\nThanks, that's fine.\nYou should sign-off on those, too: you identified the problems, told\nme the solutions, tested them, and deserve the credit.\nMoreover, Documentation/SubmittingPatches encourages it.\n\nWhen the patches are ready, I think we'll need the primary author\n(Ted) to acknowledge before the maintainer (Junio) can approve.\n\n> Luis Marsano (2):\n> git-credential-netrc: use in-tree Git.pm for tests\n> git-credential-netrc: fix exit status when tests fail\n>\n> Todd Zullinger (2):\n> git-credential-netrc: make \"all\" default target of Makefile\n> git-credential-netrc: minor whitespace cleanup in test script\n>\n> contrib/credential/netrc/Makefile | 3 +++\n> contrib/credential/netrc/t-git-credential-netrc.sh | 9 +++++----\n> contrib/credential/netrc/test.pl | 5 +++--\n> 3 files changed, 11 insertions(+), 6 deletions(-)\n"},{"id":"350050","messageId":"20180613074810.5358-1-avarab@gmail.com","threadId":"48666","inReplyTo":"CAHqJXRE8OKSKcck1APHAHccLZhox+tZi8nNu2RA74RErX8s3Pg@mail.gmail.com","subject":"[PATCH] git-credential-netrc: remove use of \"autodie\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-06-13T07:48:10Z","receivedAt":"2018-06-13T07:48:26Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The \"autodie\" module was added in Perl 5.10.1, but our INSTALL\ndocument says \"version 5.8 or later is needed\".\n\nAs discussed in <87efhfvxzu.fsf@evledraar.gmail.com> this script is in\ncontrib/, so we might not want to apply that policy, however in this\ncase \"autodie\" was recently added as a \"gratuitous safeguard\" in\n786ef50a23 (\"git-credential-netrc: accept gpg option\",\n2018-05-12) (see\n).\n\nLooking at it more carefully the addition of \"autodie\" inadvertently\nintroduced a logic error, since having it is equivalent to this patch:\n\n @@ -245,10 +244,10 @@ sub load_netrc {\n \tif ($gpgmode) {\n \t\tmy @cmd = ($options{'gpg'}, qw(--decrypt), $file);\n \t\tlog_verbose(\"Using GPG to open $file: [@cmd]\");\n -\t\topen $io, \"-|\", @cmd;\n +\t\topen $io, \"-|\", @cmd or die \"@cmd: $!\";\n \t} else {\n \t\tlog_verbose(\"Opening $file...\");\n -\t\topen $io, '<', $file;\n +\t\topen $io, '<', $file or die \"$file: $!$!;\n \t}\n\n \t# nothing to do if the open failed (we log the error later)\n\nAs shown in the context the intent of that code is not do die but to\nlog the error later.\n\nPer my reading of the file this was the only thing autodie was doing\nin this file (there was no other code it altered). So let's remove it,\nboth to fix the logic error and to get rid of the dependency.\n\n1. <87efhfvxzu.fsf@evledraar.gmail.com>\n (https://public-inbox.org/git/87efhfvxzu.fsf@evledraar.gmail.com/)\n2. \n (https://public-inbox.org/git/CAHqJXRE8OKSKcck1APHAHccLZhox+tZi8nNu2RA74RErX8s3Pg@mail.gmail.com/)\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n contrib/credential/netrc/git-credential-netrc | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc\nindex 0b9a94102e..ebfc123ec6 100755\n--- a/contrib/credential/netrc/git-credential-netrc\n+++ b/contrib/credential/netrc/git-credential-netrc\n@@ -2,7 +2,6 @@\n \n use strict;\n use warnings;\n-use autodie;\n \n use Getopt::Long;\n use File::Basename;\n-- \n2.17.0.290.gded63e768a\n\n"},{"id":"350065","messageId":"xmqqpo0u7hup.fsf@gitster-ct.c.googlers.com","threadId":"48666","inReplyTo":"20180613074810.5358-1-avarab@gmail.com","subject":"Re: [PATCH] git-credential-netrc: remove use of \"autodie\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-06-13T16:48:30Z","receivedAt":"2018-06-13T16:48:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Per my reading of the file this was the only thing autodie was doing\n> in this file (there was no other code it altered). So let's remove it,\n> both to fix the logic error and to get rid of the dependency.\n>\n> 1. <87efhfvxzu.fsf@evledraar.gmail.com>\n> (https://public-inbox.org/git/87efhfvxzu.fsf@evledraar.gmail.com/)\n> 2. \n> (https://public-inbox.org/git/CAHqJXRE8OKSKcck1APHAHccLZhox+tZi8nNu2RA74RErX8s3Pg@mail.gmail.com/)\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> contrib/credential/netrc/git-credential-netrc | 1 -\n> 1 file changed, 1 deletion(-)\n\nEven though this may not be all that release-critical, let's queue\nit so that we do not have to remember to dig it up later ;-)\n\nThank you very much to all of you involved in the thread.\n\n> diff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc\n> index 0b9a94102e..ebfc123ec6 100755\n> --- a/contrib/credential/netrc/git-credential-netrc\n> +++ b/contrib/credential/netrc/git-credential-netrc\n> @@ -2,7 +2,6 @@\n> \n> use strict;\n> use warnings;\n> -use autodie;\n> \n> use Getopt::Long;\n> use File::Basename;\n"},{"id":"350069","messageId":"20180613172111.GH3094@zaya.teonanacatl.net","threadId":"48666","inReplyTo":"CAPig+cRcPD9vh1WMX7UkiD9R_A-DrybvOYwi3R8U2bskfOV_1w@mail.gmail.com","subject":"Re: [RFC PATCH v2 2/4] git-credential-netrc: minor whitespace cleanup in test script","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2018-06-13T17:21:11Z","receivedAt":"2018-06-13T17:21:19Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Eric Sunshine wrote:\n> On Tue, Jun 12, 2018 at 11:10 PM, Todd Zullinger wrote:\n>> Signed-off-by: Todd Zullinger \n>> ---\n>> diff --git a/contrib/credential/netrc/t-git-credential-netrc.sh b/contrib/credential/netrc/t-git-credential-netrc.sh\n>> index 58191a62f8..c5661087fe 100755\n>> --- a/contrib/credential/netrc/t-git-credential-netrc.sh\n>> +++ b/contrib/credential/netrc/t-git-credential-netrc.sh\n>> @@ -17,15 +17,15 @@\n>> # set up test repository\n>>\n>> test_expect_success \\\n>> - 'set up test repository' \\\n>> - 'git config --add gpg.program test.git-config-gpg'\n>> + 'set up test repository' \\\n>> + 'git config --add gpg.program test.git-config-gpg'\n> \n> Since you're touching all the tests in this script anyhow, perhaps\n> modernize them so the title and opening quote of the test body are on\n> the same line as test_expect_success, and the closing body quote is on\n> a line of its own?\n> \n> test_expect_sucess 'setup test repository' '\n> ...test body...\n> '\n> \n> I also changed \"set up\" to \"setup\" to follow existing practice.\n> \n> (Not necessarily worth a re-roll.)\n\nThese tests were based on similar test_external tests which\nuse perl. like t0202 & t9700. Both examples use the same\nformatting (and use of 'set up'). Perhaps a later clean up\ncan adjust all three tests?\n\n-- \nTodd\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\nHow can I tell that the past isn't a fiction designed to account for\nthe discrepancy between my immediate physical sensation and my state\nof mind?\n -- Douglas Adams\n\n"},{"id":"350070","messageId":"20180613172622.GI3094@zaya.teonanacatl.net","threadId":"48666","inReplyTo":"xmqqpo0u7hup.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] git-credential-netrc: remove use of \"autodie\"","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2018-06-13T17:26:22Z","receivedAt":"2018-06-13T17:26:27Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Hi,\n\nJunio C Hamano wrote:\n> Ævar Arnfjörð Bjarmason writes:\n> \n>> Per my reading of the file this was the only thing autodie was doing\n>> in this file (there was no other code it altered). So let's remove it,\n>> both to fix the logic error and to get rid of the dependency.\n>>\n>> 1. <87efhfvxzu.fsf@evledraar.gmail.com>\n>> (https://public-inbox.org/git/87efhfvxzu.fsf@evledraar.gmail.com/)\n>> 2. \n>> (https://public-inbox.org/git/CAHqJXRE8OKSKcck1APHAHccLZhox+tZi8nNu2RA74RErX8s3Pg@mail.gmail.com/)\n>>\n>> Signed-off-by: Ævar Arnfjörð Bjarmason \n>> ---\n>> contrib/credential/netrc/git-credential-netrc | 1 -\n>> 1 file changed, 1 deletion(-)\n> \n> Even though this may not be all that release-critical, let's queue\n> it so that we do not have to remember to dig it up later ;-)\n> \n> Thank you very much to all of you involved in the thread.\n\nShould I resend the RFC v2 series as well? The only change\nI'd make would be to add my signoff on the patches which are\n'From: Luis' per suggestion.\n\nIf you think that's worth adding, I can resend or you may\nadd it while queueing. Whichever is easier for you works\nfor me.\n\nThanks,\n\n-- \nTodd\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\nAge doesn't always bring wisdom. Sometimes it comes alone.\n\n"},{"id":"350072","messageId":"CAPig+cQf1LadhKmyGoX8uYx5pAg5c3JPjxQp_=g9fibaXVAhzQ@mail.gmail.com","threadId":"48666","inReplyTo":"20180613172111.GH3094@zaya.teonanacatl.net","subject":"Re: [RFC PATCH v2 2/4] git-credential-netrc: minor whitespace cleanup in test script","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-06-13T17:43:37Z","receivedAt":"2018-06-13T17:43:52Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Jun 13, 2018 at 1:21 PM Todd Zullinger wrote:\n> Eric Sunshine wrote:\n> > Since you're touching all the tests in this script anyhow, perhaps\n> > modernize them [...]\n> > (Not necessarily worth a re-roll.)\n>\n> These tests were based on similar test_external tests which\n> use perl. like t0202 & t9700. Both examples use the same\n> formatting (and use of 'set up'). Perhaps a later clean up\n> can adjust all three tests?\n\nWhichever course of action works for you and Junio is fine. In this\ncase, it's such a minor bit of additional work to modernize the two\ntests in this script that it would make sense to do so in this patch\nif you happen to re-roll (and if you agree with me), but is itself\nprobably not worth a re-roll (as mentioned above).\n"}]}