Re: [PATCH 6/6] send-email: do not prompt for explicit repo ident
On Wed, Nov 14, 2012 at 12:05:05PM -0800, Jeff King wrote: > > When someone writes such a test, I think it could check that git > > either prompts or writes a message advising to configure the user > > email, no? Waiting until later for that seems fine to me, though. > > Yes. The problem is that the behavior and output are dependent on > factors outside the test suite, so we would have to check that one of > the possible expected outcomes happens. But I think there are really > only two such outcomes (neglecting that the ident itself can have > arbitrary content, but we do not have to check the actual content). Actually, I think the simplest thing is to add a prerequisite, like: diff --git a/t/test-lib.sh b/t/test-lib.sh index 489bc80..8d192ff 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -738,6 +738,14 @@ test_lazy_prereq UTF8_NFD_TO_NFC ' esac ' +test_lazy_prereq IMPLICIT_IDENT ' + sane_unset GIT_AUTHOR_NAME && + sane_unset GIT_AUTHOR_EMAIL && + git var GIT_AUTHOR_IDENT && + # double check that we were not polluted by config + test "$(git var GIT_AUTHOR_EXPLICIT)" = 0 +' + # When the tests are run as root, permission tests will report that # things are writable when they shouldn't be. test -w / || test_set_prereq SANITY We can't have one test machine that will cover all of the cases, but given that the test suite is run by many people across many machines, we will get coverage (and I know that some people have machines which would not pass that prereq, because I got test failure reports during the last ident refactoring). I'll include something like that in my re-roll (and it should let us test "git commit" more thoroughly, too). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] send-email: do not prompt for explicit repo ident
On Wed, Nov 14, 2012 at 09:18:27AM -0800, Jonathan Nieder wrote: > > The test scripts need to be adjusted to not expect a prompt > > for the sender, since they always have the author explicitly > > defined in the environment. Unfortunately, we cannot > > reliably test that prompting still happens in the implicit > > case, as send-email will produce inconsistent results > > depending on the machine config (if we cannot find a FQDN, > > "git var" will barf, causing us to exit early; > > At first this sounded like a bug to me --- how could the user keep > working without the sysadmin intervening? > > But then I remembered that the user can set her name and email in > .gitconfig and probably would want to in such a setup anyway. Right. They would already have to to make commits, for example. > When someone writes such a test, I think it could check that git > either prompts or writes a message advising to configure the user > email, no? Waiting until later for that seems fine to me, though. Yes. The problem is that the behavior and output are dependent on factors outside the test suite, so we would have to check that one of the possible expected outcomes happens. But I think there are really only two such outcomes (neglecting that the ident itself can have arbitrary content, but we do not have to check the actual content). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] send-email: do not prompt for explicit repo ident
Jeff King wrote: > If git-send-email is configured with sendemail.from, we will > not prompt the user for the "From" address of the emails. > If it is not configured, we prompt the user, but provide the > repo author or committer as a default. Even though we > probably have a sensible value for the default, the prompt > is a safety check in case git generated an incorrect > implicit ident string. I haven't read the code carefully, but this behavior sounds sensible, so for what it's worth, Acked-by: Jonathan Nieder [...] > The test scripts need to be adjusted to not expect a prompt > for the sender, since they always have the author explicitly > defined in the environment. Unfortunately, we cannot > reliably test that prompting still happens in the implicit > case, as send-email will produce inconsistent results > depending on the machine config (if we cannot find a FQDN, > "git var" will barf, causing us to exit early; At first this sounded like a bug to me --- how could the user keep working without the sysadmin intervening? But then I remembered that the user can set her name and email in .gitconfig and probably would want to in such a setup anyway. When someone writes such a test, I think it could check that git either prompts or writes a message advising to configure the user email, no? Waiting until later for that seems fine to me, though. Thanks, Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/6] send-email: do not prompt for explicit repo ident
If git-send-email is configured with sendemail.from, we will
not prompt the user for the "From" address of the emails.
If it is not configured, we prompt the user, but provide the
repo author or committer as a default. Even though we
probably have a sensible value for the default, the prompt
is a safety check in case git generated an incorrect
implicit ident string.
Now that Git.pm will tell us whether the ident is implicit or
explicit, we can stop prompting in the explicit case, saving
most users from having to see the prompt at all.
The test scripts need to be adjusted to not expect a prompt
for the sender, since they always have the author explicitly
defined in the environment. Unfortunately, we cannot
reliably test that prompting still happens in the implicit
case, as send-email will produce inconsistent results
depending on the machine config (if we cannot find a FQDN,
"git var" will barf, causing us to exit early; if we can,
send-email will continue but prompt).
Signed-off-by: Jeff King
---
git-send-email.perl | 22 +-
t/t9001-send-email.sh | 5 ++---
2 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 5a7c29d..0c49b32 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -436,9 +436,8 @@ if (0) {
}
}
-my ($repoauthor, $repocommitter);
-($repoauthor) = Git::ident_person(@repo, 'author');
-($repocommitter) = Git::ident_person(@repo, 'committer');
+my ($repoauthor, $author_explicit) = Git::ident_person(@repo, 'author');
+my ($repocommitter, $committer_explicit) = Git::ident_person(@repo,
'committer');
# Verify the user input
@@ -755,12 +754,17 @@ if (!$force) {
my $prompting = 0;
if (!defined $sender) {
- $sender = $repoauthor || $repocommitter || '';
- $sender = ask("Who should the emails appear to be from? [$sender] ",
- default => $sender,
- valid_re => qr/\@.*\./, confirm_only => 1);
- print "Emails will be sent from: ", $sender, "\n";
- $prompting++;
+ ($sender, my $explicit) =
+ defined $repoauthor ? ($repoauthor, $author_explicit) :
+ defined $repocommitter ? ($repocommitter, $committer_explicit) :
+ ('', 0);
+ if (!$explicit) {
+ $sender = ask("Who should the emails appear to be from?
[$sender] ",
+ default => $sender,
+ valid_re => qr/\@.*\./, confirm_only => 1);
+ print "Emails will be sent from: ", $sender, "\n";
+ $prompting++;
+ }
}
if (!@initial_to && !defined $to_cmd) {
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 6c6af7d..c5d66cf 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -191,14 +191,13 @@ test_expect_success $PREREQ 'Show all headers' '
test_expect_success $PREREQ 'Prompting works' '
clean_fake_sendmail &&
- (echo "Example "
-echo "t...@example.com"
+ (echo "t...@example.com"
echo ""
) | GIT_SEND_EMAIL_NOTTY=1 git send-email \
--smtp-server="$(pwd)/fake.sendmail" \
$patches \
2>errors &&
- grep "^From: Example \$" msgtxt1 &&
+ grep "^From: A U Thor \$" msgtxt1 &&
grep "^To: t...@example.com\$" msgtxt1
'
--
1.8.0.207.gdf2154c
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
