* git-send-email no longer works outside a repository?
@ 2017-06-01 22:45 Jacob Keller
2017-06-01 23:00 ` Brandon Williams
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Jacob Keller @ 2017-06-01 22:45 UTC (permalink / raw)
To: Git mailing list
I often use git-send-email in order to send patch files. Recently when
I tried to do this outside a repository I got some cryptic failures,
I'm using the master branch, git version 2.13.0.311.g0339965c70d6
I generate the patch files and copy them into a separate folder
outside of the repository, and make sure everything looks good and
write a cover letter, then I try to send them with
$git send-email --to=<address> 00*
Can't call method "repo_path" on an undefined value at
/home/jekeller/libexec/git-core/git-send-email line 1759.
Even weirder, if I move into the repository and try to send files
which are outside, such as:
$git send-email --to=iwl<address> ../patches/00*
fatal: /home/jekeller/git/patches/00*:
'/home/jekeller/git/patches/00*' is outside repository
format-patch -o /tmp/AZatqXB1uD /home/jekeller/git/patches/00*:
command returned error: 128
I would expect that if you're outside a repository the command (as
before) would alllow you to send files. It shouldn't strictly need to
be inside a repository to function.
I found this first on pu, but as above, I checked out master and still
seem to have the problem.
I'm working on a bisect now.
Thanks,
Jake
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: git-send-email no longer works outside a repository? 2017-06-01 22:45 git-send-email no longer works outside a repository? Jacob Keller @ 2017-06-01 23:00 ` Brandon Williams 2017-06-01 23:14 ` Junio C Hamano 2017-06-01 23:50 ` [PATCH] send-email: check for repo before invoking hook Jonathan Tan 2 siblings, 0 replies; 7+ messages in thread From: Brandon Williams @ 2017-06-01 23:00 UTC (permalink / raw) To: Jacob Keller; +Cc: Git mailing list, jonathantanmy On 06/01, Jacob Keller wrote: > I often use git-send-email in order to send patch files. Recently when > I tried to do this outside a repository I got some cryptic failures, > I'm using the master branch, git version 2.13.0.311.g0339965c70d6 > > I generate the patch files and copy them into a separate folder > outside of the repository, and make sure everything looks good and > write a cover letter, then I try to send them with > > $git send-email --to=<address> 00* > Can't call method "repo_path" on an undefined value at > /home/jekeller/libexec/git-core/git-send-email line 1759. > > Even weirder, if I move into the repository and try to send files > which are outside, such as: > > $git send-email --to=iwl<address> ../patches/00* > fatal: /home/jekeller/git/patches/00*: > '/home/jekeller/git/patches/00*' is outside repository > format-patch -o /tmp/AZatqXB1uD /home/jekeller/git/patches/00*: > command returned error: 128 > > I would expect that if you're outside a repository the command (as > before) would alllow you to send files. It shouldn't strictly need to > be inside a repository to function. > > I found this first on pu, but as above, I checked out master and still > seem to have the problem. > > I'm working on a bisect now. > > Thanks, > Jake This looks like it is due to '6489660b4 (origin/jt/send-email-validate-hook) send-email: support validate hook'. It's trying to look for a hook in a non-existent repository. -- Brandon Williams ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: git-send-email no longer works outside a repository? 2017-06-01 22:45 git-send-email no longer works outside a repository? Jacob Keller 2017-06-01 23:00 ` Brandon Williams @ 2017-06-01 23:14 ` Junio C Hamano 2017-06-01 23:50 ` [PATCH] send-email: check for repo before invoking hook Jonathan Tan 2 siblings, 0 replies; 7+ messages in thread From: Junio C Hamano @ 2017-06-01 23:14 UTC (permalink / raw) To: Jacob Keller; +Cc: Git mailing list, Jonathan Tan Jacob Keller <jacob.keller@gmail.com> writes: > I often use git-send-email in order to send patch files. Recently when > I tried to do this outside a repository I got some cryptic failures, > I'm using the master branch, git version 2.13.0.311.g0339965c70d6 > > I generate the patch files and copy them into a separate folder > outside of the repository, and make sure everything looks good and > write a cover letter, then I try to send them with > > $git send-email --to=<address> 00* > Can't call method "repo_path" on an undefined value at > /home/jekeller/libexec/git-core/git-send-email line 1759. > > Even weirder, if I move into the repository and try to send files > which are outside, such as: > > $git send-email --to=iwl<address> ../patches/00* > fatal: /home/jekeller/git/patches/00*: > '/home/jekeller/git/patches/00*' is outside repository > format-patch -o /tmp/AZatqXB1uD /home/jekeller/git/patches/00*: > command returned error: 128 > > I would expect that if you're outside a repository the command (as > before) would alllow you to send files. It shouldn't strictly need to > be inside a repository to function. > > I found this first on pu, but as above, I checked out master and still > seem to have the problem. > > I'm working on a bisect now. This certainly is not an intended change. That validate-hook thing must be made optional and conditional to the existence of a repository. Thanks for reporting. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] send-email: check for repo before invoking hook 2017-06-01 22:45 git-send-email no longer works outside a repository? Jacob Keller 2017-06-01 23:00 ` Brandon Williams 2017-06-01 23:14 ` Junio C Hamano @ 2017-06-01 23:50 ` Jonathan Tan 2017-06-02 0:22 ` Todd Zullinger 2017-06-02 1:59 ` Junio C Hamano 2 siblings, 2 replies; 7+ messages in thread From: Jonathan Tan @ 2017-06-01 23:50 UTC (permalink / raw) To: git; +Cc: Jonathan Tan, bmwill, gitster, jacob.keller Unless --no-validate is passed, send-email will invoke $repo->repo_path() in its search for a validate hook regardless of whether a Git repo is actually present. Teach send-email to first check for repo existence. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> --- Thanks for the notification. Here's a patch to fix that. --- git-send-email.perl | 32 +++++++++++++++++--------------- t/t9001-send-email.sh | 8 ++++++++ 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/git-send-email.perl b/git-send-email.perl index f0417f64e..94c54dc5a 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -1755,21 +1755,23 @@ sub unique_email_list { sub validate_patch { my $fn = shift; - my $validate_hook = catfile(catdir($repo->repo_path(), 'hooks'), - 'sendemail-validate'); - my $hook_error; - if (-x $validate_hook) { - my $target = abs_path($fn); - # The hook needs a correct cwd and GIT_DIR. - my $cwd_save = cwd(); - chdir($repo->wc_path() or $repo->repo_path()) - or die("chdir: $!"); - local $ENV{"GIT_DIR"} = $repo->repo_path(); - $hook_error = "rejected by sendemail-validate hook" - if system($validate_hook, $target); - chdir($cwd_save) or die("chdir: $!"); - } - return $hook_error if $hook_error; + if ($repo) { + my $validate_hook = catfile(catdir($repo->repo_path(), 'hooks'), + 'sendemail-validate'); + my $hook_error; + if (-x $validate_hook) { + my $target = abs_path($fn); + # The hook needs a correct cwd and GIT_DIR. + my $cwd_save = cwd(); + chdir($repo->wc_path() or $repo->repo_path()) + or die("chdir: $!"); + local $ENV{"GIT_DIR"} = $repo->repo_path(); + $hook_error = "rejected by sendemail-validate hook" + if system($validate_hook, $target); + chdir($cwd_save) or die("chdir: $!"); + } + return $hook_error if $hook_error; + } open(my $fh, '<', $fn) or die sprintf(__("unable to open %s: %s\n"), $fn, $!); diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 15128c755..d1e4e8ad1 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -1953,4 +1953,12 @@ test_expect_success $PREREQ 'invoke hook' ' ) ' +test_expect_success $PREREQ 'test that send-email works outside a repo' ' + nongit git send-email \ + --from="Example <nobody@example.com>" \ + --to=nobody@example.com \ + --smtp-server="$(pwd)/fake.sendmail" \ + "$(pwd)/0001-add-master.patch" +' + test_done -- 2.13.0.506.g27d5fe0cd-goog ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] send-email: check for repo before invoking hook 2017-06-01 23:50 ` [PATCH] send-email: check for repo before invoking hook Jonathan Tan @ 2017-06-02 0:22 ` Todd Zullinger 2017-06-02 0:49 ` Jacob Keller 2017-06-02 1:59 ` Junio C Hamano 1 sibling, 1 reply; 7+ messages in thread From: Todd Zullinger @ 2017-06-02 0:22 UTC (permalink / raw) To: Jonathan Tan; +Cc: git, bmwill, gitster, jacob.keller Hi Jonathan, Jonathan Tan wrote: > Thanks for the notification. Here's a patch to fix that. > --- > git-send-email.perl | 32 +++++++++++++++++--------------- > t/t9001-send-email.sh | 8 ++++++++ > 2 files changed, 25 insertions(+), 15 deletions(-) > > diff --git a/git-send-email.perl b/git-send-email.perl > index f0417f64e..94c54dc5a 100755 > --- a/git-send-email.perl > +++ b/git-send-email.perl > @@ -1755,21 +1755,23 @@ sub unique_email_list { > sub validate_patch { > my $fn = shift; > > - my $validate_hook = catfile(catdir($repo->repo_path(), 'hooks'), > - 'sendemail-validate'); > - my $hook_error; > - if (-x $validate_hook) { > - my $target = abs_path($fn); > - # The hook needs a correct cwd and GIT_DIR. > - my $cwd_save = cwd(); > - chdir($repo->wc_path() or $repo->repo_path()) > - or die("chdir: $!"); > - local $ENV{"GIT_DIR"} = $repo->repo_path(); > - $hook_error = "rejected by sendemail-validate hook" > - if system($validate_hook, $target); > - chdir($cwd_save) or die("chdir: $!"); > - } > - return $hook_error if $hook_error; > + if ($repo) { > + my $validate_hook = catfile(catdir($repo->repo_path(), 'hooks'), > + 'sendemail-validate'); > + my $hook_error; > + if (-x $validate_hook) { > + my $target = abs_path($fn); > + # The hook needs a correct cwd and GIT_DIR. > + my $cwd_save = cwd(); > + chdir($repo->wc_path() or $repo->repo_path()) > + or die("chdir: $!"); > + local $ENV{"GIT_DIR"} = $repo->repo_path(); > + $hook_error = "rejected by sendemail-validate hook" > + if system($validate_hook, $target); > + chdir($cwd_save) or die("chdir: $!"); > + } > + return $hook_error if $hook_error; > + } Would it be worth doing the $repo test when defining $validate_hook to avoid a layer of indentation? Something like this (with whatever proper wrapping/indentation is used for perl, if I have that wildly incorrect, of course)? -- >8 -- diff --git i/git-send-email.perl w/git-send-email.perl index f0417f64e7..e78a0a728a 100755 --- i/git-send-email.perl +++ w/git-send-email.perl @@ -1755,8 +1755,9 @@ sub unique_email_list { sub validate_patch { my $fn = shift; - my $validate_hook = catfile(catdir($repo->repo_path(), 'hooks'), - 'sendemail-validate'); + my $validate_hook = $repo ? + catfile(catdir($repo->repo_path(), 'hooks'), + 'sendemail-validate') : ''; my $hook_error; if (-x $validate_hook) { my $target = abs_path($fn); -- Todd ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ I have to decide between two equally frightening options. If I wanted to do that, I'd vote. -- Duckman ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] send-email: check for repo before invoking hook 2017-06-02 0:22 ` Todd Zullinger @ 2017-06-02 0:49 ` Jacob Keller 0 siblings, 0 replies; 7+ messages in thread From: Jacob Keller @ 2017-06-02 0:49 UTC (permalink / raw) To: Todd Zullinger Cc: Jonathan Tan, Git mailing list, Brandon Williams, Junio C Hamano On Thu, Jun 1, 2017 at 5:22 PM, Todd Zullinger <tmz@pobox.com> wrote: > Hi Jonathan, > > Jonathan Tan wrote: >> >> Thanks for the notification. Here's a patch to fix that. --- >> git-send-email.perl | 32 +++++++++++++++++--------------- >> t/t9001-send-email.sh | 8 ++++++++ 2 files changed, 25 insertions(+), 15 >> deletions(-) >> >> diff --git a/git-send-email.perl b/git-send-email.perl index >> f0417f64e..94c54dc5a 100755 --- a/git-send-email.perl +++ >> b/git-send-email.perl @@ -1755,21 +1755,23 @@ sub unique_email_list { sub >> validate_patch { my $fn = shift; >> >> - my $validate_hook = catfile(catdir($repo->repo_path(), 'hooks'), - >> 'sendemail-validate'); - my $hook_error; - if (-x $validate_hook) { >> - my $target = abs_path($fn); - # The hook needs a >> correct cwd and GIT_DIR. - my $cwd_save = cwd(); - >> chdir($repo->wc_path() or $repo->repo_path()) - or >> die("chdir: $!"); - local $ENV{"GIT_DIR"} = $repo->repo_path(); - >> $hook_error = "rejected by sendemail-validate hook" - if >> system($validate_hook, $target); - chdir($cwd_save) or die("chdir: >> $!"); - } - return $hook_error if $hook_error; + if ($repo) { + >> my $validate_hook = catfile(catdir($repo->repo_path(), 'hooks'), + >> 'sendemail-validate'); + my $hook_error; + if (-x >> $validate_hook) { + my $target = abs_path($fn); + >> # The hook needs a correct cwd and GIT_DIR. + my $cwd_save >> = cwd(); + chdir($repo->wc_path() or $repo->repo_path()) + >> or die("chdir: $!"); + local $ENV{"GIT_DIR"} = >> $repo->repo_path(); + $hook_error = "rejected by >> sendemail-validate hook" + if >> system($validate_hook, $target); + chdir($cwd_save) or >> die("chdir: $!"); + } + return $hook_error if >> $hook_error; + } > > > Would it be worth doing the $repo test when defining $validate_hook to avoid > a layer of indentation? Something like this (with whatever proper > wrapping/indentation is used for perl, if I have that wildly incorrect, of > course)? > This approach makes more sense to me, but either should fix the bug. The first approach might be more resilient against future changes though...? Thanks, Jake > -- >8 -- > diff --git i/git-send-email.perl w/git-send-email.perl > index f0417f64e7..e78a0a728a 100755 > --- i/git-send-email.perl > +++ w/git-send-email.perl > @@ -1755,8 +1755,9 @@ sub unique_email_list { > sub validate_patch { > my $fn = shift; > > - my $validate_hook = catfile(catdir($repo->repo_path(), 'hooks'), > - 'sendemail-validate'); > + my $validate_hook = $repo ? > + catfile(catdir($repo->repo_path(), 'hooks'), > + 'sendemail-validate') : ''; > my $hook_error; > if (-x $validate_hook) { > my $target = abs_path($fn); > > -- > Todd > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > I have to decide between two equally frightening options. If I wanted > to do that, I'd vote. > -- Duckman > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] send-email: check for repo before invoking hook 2017-06-01 23:50 ` [PATCH] send-email: check for repo before invoking hook Jonathan Tan 2017-06-02 0:22 ` Todd Zullinger @ 2017-06-02 1:59 ` Junio C Hamano 1 sibling, 0 replies; 7+ messages in thread From: Junio C Hamano @ 2017-06-02 1:59 UTC (permalink / raw) To: Jonathan Tan; +Cc: git, bmwill, jacob.keller Thanks. "git show -w" tells readers how this fix is trivially correct ;-) Will apply. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-06-02 1:59 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-06-01 22:45 git-send-email no longer works outside a repository? Jacob Keller 2017-06-01 23:00 ` Brandon Williams 2017-06-01 23:14 ` Junio C Hamano 2017-06-01 23:50 ` [PATCH] send-email: check for repo before invoking hook Jonathan Tan 2017-06-02 0:22 ` Todd Zullinger 2017-06-02 0:49 ` Jacob Keller 2017-06-02 1:59 ` Junio C Hamano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).