* [PATCH 2/2] [RFC] push: allow delete one level ref
From: ZheNing Hu via GitGitGadget @ 2023-01-17 10:32 UTC (permalink / raw)
To: git
Cc: Ævar Arnfjörð Bjarmason, Junio C Hamano,
Derrick Stolee, Elijah Newren, Michael Haggerty, Christian Couder,
Jeff King, ZheNing Hu, ZheNing Hu
In-Reply-To: <pull.1465.git.1673951562.gitgitgadget@gmail.com>
From: ZheNing Hu <adlternative@gmail.com>
Git will reject the deletion of one level refs e,g. "refs/foo"
through "git push -d", however, some users want to be able to
clean up these branches that were created unexpectedly on the
remote.
Therefore, when updating branches on the server with
"git receive-pack", by checking whether it is a branch deletion
operation, it will determine whether to allow the update of
one level refs. This avoids creating/updating such one level
branches, but allows them to be deleted.
On the client side, "git push" also does not properly fill in
the old-oid of one level refs, which causes the server-side
"git receive-pack" to think that the ref's old-oid has changed
when deleting one level refs, this causes the push to be rejected.
So the solution is to fix the client to be able to delete
one level refs by properly filling old-oid.
Signed-off-by: ZheNing Hu <adlternative@gmail.com>
---
builtin/receive-pack.c | 5 ++++-
connect.c | 2 +-
t/t5516-fetch-push.sh | 13 +++++++++++++
3 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 13ff9fae3ba..ad21877ea1b 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1463,7 +1463,10 @@ static const char *update(struct command *cmd, struct shallow_info *si)
find_shared_symref(worktrees, "HEAD", name);
/* only refs/... are allowed */
- if (!starts_with(name, "refs/") || check_refname_format(name + 5, 0)) {
+ if (!starts_with(name, "refs/") ||
+ check_refname_format(name + 5,
+ is_null_oid(new_oid) ?
+ REFNAME_ALLOW_ONELEVEL : 0)) {
rp_error("refusing to update funny ref '%s' remotely", name);
ret = "funny refname";
goto out;
diff --git a/connect.c b/connect.c
index 63e59641c0d..b841ae58e03 100644
--- a/connect.c
+++ b/connect.c
@@ -30,7 +30,7 @@ static int check_ref(const char *name, unsigned int flags)
return 0;
/* REF_NORMAL means that we don't want the magic fake tag refs */
- if ((flags & REF_NORMAL) && check_refname_format(name, 0))
+ if ((flags & REF_NORMAL) && check_refname_format(name, REFNAME_ALLOW_ONELEVEL))
return 0;
/* REF_HEADS means that we want regular branch heads */
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index f37861efc40..dec8950a392 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -903,6 +903,19 @@ test_expect_success 'push --delete refuses empty string' '
test_must_fail git push testrepo --delete ""
'
+test_expect_success 'push --delete onelevel refspecs' '
+ mk_test testrepo heads/main &&
+ (
+ cd testrepo &&
+ git update-ref refs/onelevel refs/heads/main
+ ) &&
+ git push testrepo --delete refs/onelevel &&
+ (
+ cd testrepo &&
+ test_must_fail git rev-parse --verify refs/onelevel
+ )
+'
+
test_expect_success 'warn on push to HEAD of non-bare repository' '
mk_test testrepo heads/main &&
(
--
gitgitgadget
^ permalink raw reply related
* [PATCH 0/2] [RFC] push: allow delete one level ref
From: ZheNing Hu via GitGitGadget @ 2023-01-17 10:32 UTC (permalink / raw)
To: git
Cc: Ævar Arnfjörð Bjarmason, Junio C Hamano,
Derrick Stolee, Elijah Newren, Michael Haggerty, Christian Couder,
Jeff King, ZheNing Hu
This might be an odd request: allow git push to delete one level refs like
"ref/foo".
Some users on my side often push "refs/for/master" to the remote for code
review, but due to a user's typo, "refs/master" is pushed to the remote.
Pushing a one level ref like "refs/foo" should not have been successful, but
since my server side directly updated the ref during the proc-receive-hook
phase of git receive-pack, "refs/foo" was mistakenly left at on the server.
But for some reasons, users cannot delete this special branch through "git
push -d".
First, I executed git update-ref --stdin inside my proc-receive-hook to
delete the branch. As a result, update-ref reported an error: "cannot lock
ref 'refs/foo': reference already exists".
So I tried GIT_TRACKET_PACKET to debug and found that git push did not seem
to pass the correct ref old-oid, which is why update-ref reported an error.
"18:13:41.214872 pkt-line.c:80 packet: receive-pack< 0000000000000000000000000000000000000000 0000000000000000000000000000000000000000 refs/foo\0 report-status-v2 side-band-64k object-format=sha1 agent=git/2.39.0.227.g262c45b6a1"
Tracing it back, the check_ref() in the git push link didn't record the
old-oid for the remote "refs/foo".
At the same time, the server update() process will reject the one level ref
by default and return a "funny refname" error.
It is worth mentioning that since I deleted the branch, the error message
returned here is "refusing to create funny ref 'refs/foo' remotely", which
is also worth fixing.
So this patch series do:
v1.
1. fix funny refname error message from "create" to "update".
2. let server allow delete one level ref.
3. let client pass correct one level ref old-oid.
ZheNing Hu (2):
receive-pack: fix funny ref error messsage
[RFC] push: allow delete one level ref
builtin/receive-pack.c | 7 +++++--
connect.c | 2 +-
t/t5516-fetch-push.sh | 18 ++++++++++++++++++
3 files changed, 24 insertions(+), 3 deletions(-)
base-commit: a38d39a4c50d1275833aba54c4dbdfce9e2e9ca1
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1465%2Fadlternative%2Fzh%2Fdelete-one-level-ref-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1465/adlternative/zh/delete-one-level-ref-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1465
--
gitgitgadget
^ permalink raw reply
* [PATCH 1/2] receive-pack: fix funny ref error messsage
From: ZheNing Hu via GitGitGadget @ 2023-01-17 10:32 UTC (permalink / raw)
To: git
Cc: Ævar Arnfjörð Bjarmason, Junio C Hamano,
Derrick Stolee, Elijah Newren, Michael Haggerty, Christian Couder,
Jeff King, ZheNing Hu, ZheNing Hu
In-Reply-To: <pull.1465.git.1673951562.gitgitgadget@gmail.com>
From: ZheNing Hu <adlternative@gmail.com>
When the user deletes the remote one level branch through
"git push origin -d refs/foo", remote will return an error:
"refusing to create funny ref 'refs/foo' remotely", here we
are not creating "refs/foo" instead wants to delete it, so a
better error description here would be: "refusing to update
funny ref 'refs/foo' remotely".
Signed-off-by: ZheNing Hu <adlternative@gmail.com>
---
builtin/receive-pack.c | 2 +-
t/t5516-fetch-push.sh | 5 +++++
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index a90af303630..13ff9fae3ba 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1464,7 +1464,7 @@ static const char *update(struct command *cmd, struct shallow_info *si)
/* only refs/... are allowed */
if (!starts_with(name, "refs/") || check_refname_format(name + 5, 0)) {
- rp_error("refusing to create funny ref '%s' remotely", name);
+ rp_error("refusing to update funny ref '%s' remotely", name);
ret = "funny refname";
goto out;
}
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 98a27a2948b..f37861efc40 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -401,6 +401,11 @@ test_expect_success 'push with ambiguity' '
'
+test_expect_success 'push with onelevel ref' '
+ mk_test testrepo heads/main &&
+ test_must_fail git push testrepo HEAD:refs/onelevel
+'
+
test_expect_success 'push with colon-less refspec (1)' '
mk_test testrepo heads/frotz tags/frotz &&
--
gitgitgadget
^ permalink raw reply related
* GSoC 2023
From: Christian Couder @ 2023-01-17 9:34 UTC (permalink / raw)
To: git
Hi everyone,
GSoC Org Applications open next week on Monday, January 23rd at 1800
UTC and close on Tuesday, February 7th at 1800 UTC.
I am interested in mentoring and being an org admin for Git again this
year, so I plan to apply for Git soon.
The GSoC contributor application deadline is April 4 - 18:00 UTC, so
(co-)mentors and org admins are already welcome to volunteer. We also
need project ideas to refresh our idea page from last year
(https://git.github.io/SoC-2022-Ideas/).
There will be a GSoC Meetup in Brussels during FOSDEM weekend on
Saturday, February 4th in the evening. I won't be there but if you are
around, interested and haven't received the link to register directly
from Google, let me know so I can send it to you.
Best,
Christian.
^ permalink raw reply
* Re: [PATCH v6 2/2] send-email: expose header information to git-send-email's sendemail-validate hook
From: Junio C Hamano @ 2023-01-17 7:31 UTC (permalink / raw)
To: Luben Tuikov; +Cc: Strawbridge, Michael, git@vger.kernel.org
In-Reply-To: <3a2d4559-fce2-80f3-bafd-5eb8ac1a7eff@amd.com>
Luben Tuikov <luben.tuikov@amd.com> writes:
>> +test_expect_success $PREREQ "--validate hook supports header argument" '
>> + write_script my-hooks/sendemail-validate <<-\EOF &&
>> + if test -s "$2"
>> + then
>> + cat "$2" >actual
>> + exit 1
>> + fi
>> + EOF
If "$2" is not given, or an empty "$2" is given, is that an error?
I am wondering if the lack of "else" clause (and the hook exits with
success when "$2" is an empty file) here is intentional.
>> + cat actual | replace_variable_fields \
>> + >actual-headers &&
Do not cat a single file into a pipe. You can instead redirect out
of the file to whatever is reading from the pipe. I.e.
replace_variable_fields <actual >actual-headers &&
>> + test_cmp expected-headers actual-headers
>> +'
OK. We make sure the presence and the order of the fields in the
output just like all the other tests in this file do (which I think
may be a bit too much---there is no strong reason to insist that
"Subject:" comes before or after "Date:" or is spelled "Subject:"
and not "subject:" or "SUBJECT:"---but that is a problem shared with
many other existing tests in this file and this patch is not making
it much worse).
>> for enc in 7bit 8bit quoted-printable base64
>> do
>> test_expect_success $PREREQ "--transfer-encoding=$enc produces correct header" '
>
> As Junio and I discussed in the v5 2/2 patch review, here we may want to
> do something like this: Add a custom header to the SMTP envelope and then make
> sure that that is present when the hook checks $2.
Adding a custom header test is also fine, but I am OK with what we
see above, to verify the headers just the same way as existing
tests.
^ permalink raw reply
* Re: [PATCH v6 2/2] send-email: expose header information to git-send-email's sendemail-validate hook
From: Luben Tuikov @ 2023-01-17 5:06 UTC (permalink / raw)
To: Strawbridge, Michael, git@vger.kernel.org; +Cc: Junio C Hamano
In-Reply-To: <20230117013932.47570-3-michael.strawbridge@amd.com>
Hi Michael,
On 2023-01-16 20:39, Strawbridge, Michael wrote:
--[cut]--
> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
> index 1130ef21b3..346ff1463e 100755
> --- a/t/t9001-send-email.sh
> +++ b/t/t9001-send-email.sh
> @@ -540,7 +540,7 @@ test_expect_success $PREREQ "--validate respects relative core.hooksPath path" '
> test_path_is_file my-hooks.ran &&
> cat >expect <<-EOF &&
> fatal: longline.patch: rejected by sendemail-validate hook
> - fatal: command '"'"'git hook run --ignore-missing sendemail-validate -- <patch>'"'"' died with exit code 1
> + fatal: command '"'"'git hook run --ignore-missing sendemail-validate -- <patch> <header>'"'"' died with exit code 1
> warning: no patches were sent
> EOF
> test_cmp expect actual
> @@ -559,12 +559,55 @@ test_expect_success $PREREQ "--validate respects absolute core.hooksPath path" '
> test_path_is_file my-hooks.ran &&
> cat >expect <<-EOF &&
> fatal: longline.patch: rejected by sendemail-validate hook
> - fatal: command '"'"'git hook run --ignore-missing sendemail-validate -- <patch>'"'"' died with exit code 1
> + fatal: command '"'"'git hook run --ignore-missing sendemail-validate -- <patch> <header>'"'"' died with exit code 1
> warning: no patches were sent
> EOF
> test_cmp expect actual
> '
>
> +test_expect_success $PREREQ 'setup expect' "
> +cat >expected-headers <<\EOF
> +From: Example <from@example.com>
> +To: to@example.com
> +Cc: cc@example.com,
> + A <author@example.com>,
> + One <one@example.com>,
> + two@example.com
> +Subject: [PATCH 1/1] Second.
> +Date: DATE-STRING
> +Message-Id: MESSAGE-ID-STRING
> +X-Mailer: X-MAILER-STRING
> +Reply-To: Reply <reply@example.com>
> +MIME-Version: 1.0
> +Content-Transfer-Encoding: quoted-printable
> +EOF
> +"
> +
> +test_expect_success $PREREQ "--validate hook supports header argument" '
> + write_script my-hooks/sendemail-validate <<-\EOF &&
> + if test -s "$2"
> + then
> + cat "$2" >actual
> + exit 1
> + fi
> + EOF
> + test_config core.hooksPath "my-hooks" &&
> + test_must_fail git send-email \
> + --dry-run \
> + --suppress-cc=sob \
> + --from="Example <from@example.com>" \
> + --reply-to="Reply <reply@example.com>" \
> + --to=to@example.com \
> + --cc=cc@example.com \
> + --bcc=bcc@example.com \
> + --smtp-server="$(pwd)/fake.sendmail" \
> + --validate \
> + longline.patch &&
> + cat actual | replace_variable_fields \
> + >actual-headers &&
> + test_cmp expected-headers actual-headers
> +'
> +
> for enc in 7bit 8bit quoted-printable base64
> do
> test_expect_success $PREREQ "--transfer-encoding=$enc produces correct header" '
As Junio and I discussed in the v5 2/2 patch review, here we may want to
do something like this: Add a custom header to the SMTP envelope and then make
sure that that is present when the hook checks $2.
To add a custom header, (and this uses real-world data, which is good),
use the following:
git format-patch --stdout --add-header="X-test-header: v1.0" HEAD^..HEAD > /tmp/some-temp-file
Then the hook verifies that "X-test-header: v1.0" is present in $2, when git-send-email
is run with /tmp/some-temp-file.
--
Regards,
Luben
^ permalink raw reply
* Re: [PATCH v5 2/2] send-email: expose header information to git-send-email's sendemail-validate hook
From: Luben Tuikov @ 2023-01-17 4:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Strawbridge, Michael, git@vger.kernel.org
In-Reply-To: <xmqqmt6hbx6y.fsf@gitster.g>
On 2023-01-16 23:29, Junio C Hamano wrote:
> Luben Tuikov <luben.tuikov@amd.com> writes:
>
>> We're generally not interested in "what else" is in the SMTP envelope
>> and headers.
>> ...
>> The idea is that hook writers would merely be grepping for a particular
>> header they're interested in--it could even be a custom header, "X-something"
>> for instance, and if present, they'll check the contents of that header and
>> validate the patch, or perform some other action.
>
> I am following you thus far, but ...
>
>> So, checking that the SMTP envelope and headers, $2, is not empty suffices
>> for what this patch set implements. We leave it up to the hook writers to
>> inspect the SMTP envelope and headers for their particular hook purpose.
>
> ... I am lost here. To make sure that the hook writers' grepping
> for a particular contents in the file $2 does find what they are
> trying to find, wouldn't we want to have a test that looks for a
> "known" header that should exist in the expected output (or even
> better, arrange the send-email invocation so that a custom header is
> injected to the output of format-patch and grep for that "known"
> header)?
Yes, that's exactly how my Git is set up. (It's a bit more involved
than this, but essentially a custom header is added and a check performed.)
I'll follow up on v6 2/2 patch review with this.
--
Regards,
Luben
^ permalink raw reply
* Re: [PATCH v6 2/2] send-email: expose header information to git-send-email's sendemail-validate hook
From: Luben Tuikov @ 2023-01-17 4:35 UTC (permalink / raw)
To: Strawbridge, Michael, git@vger.kernel.org; +Cc: Junio C Hamano
In-Reply-To: <20230117013932.47570-3-michael.strawbridge@amd.com>
Hi Michael,
Good work on this. I've a few tiny notes following.
On 2023-01-16 20:39, Strawbridge, Michael wrote:
> To allow further flexibility in the git hook, the SMTP header
"git" is something different. You want to use the capitalization "Git".
> information of the email that git-send-email intends to send, is now
"that" --> "which".
> passed as a 2nd argument to the sendemail-validate hook.
"a 2nd argument" --> "the 2nd argument".
> As an example, this can be useful for acting upon keywords in the
> subject or specific email addresses.
>
> Cc: Luben Tuikov <luben.tuikov@amd.com>
> Cc: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Michael Strawbridge <michael.strawbridge@amd.com>
> ---
> Documentation/githooks.txt | 29 +++++++++++++++++++----
> git-send-email.perl | 31 +++++++++++++++++--------
> t/t9001-send-email.sh | 47 ++++++++++++++++++++++++++++++++++++--
> 3 files changed, 91 insertions(+), 16 deletions(-)
>
> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
> index a16e62bc8c..e80f481efd 100644
> --- a/Documentation/githooks.txt
> +++ b/Documentation/githooks.txt
> @@ -583,10 +583,31 @@ processed by rebase.
> sendemail-validate
> ~~~~~~~~~~~~~~~~~~
>
> -This hook is invoked by linkgit:git-send-email[1]. It takes a single parameter,
> -the name of the file that holds the e-mail to be sent. Exiting with a
> -non-zero status causes `git send-email` to abort before sending any
> -e-mails.
> +This hook is invoked by linkgit:git-send-email[1].
> +
> +It takes these command line arguments:
"It takes two command line arguments. They are,"
> +1. the name of the file that holds the e-mail to be sent.
"which holds the contents of the email to be sent."
Sentence ends and the next one should be capitalized.
> +2. the name of the file that holds the SMTP headers to be used.
"The name of the file which holds the SMTP envelope and headers of the email."
> +
> +The SMTP headers will be passed to the hook in the below format.
> +Take notice of the capitalization and multi-line tab structure.
Always use present simple tense when describing mechanics of code,
not future tense, "are passed". Think of when the user is reading
this long after the patch went in.
Also, please use "the format below."
> + From: Example <from@example.com>
> + To: to@example.com
> + Cc: cc@example.com,
> + A <author@example.com>,
> + One <one@example.com>,
> + two@example.com
> + Subject: PATCH-STRING
> + Date: DATE-STRING
> + Message-Id: MESSAGE-ID-STRING
> + X-Mailer: X-MAILER-STRING
> + Reply-To: Reply <reply@example.com>
> + MIME-Version: 1.0
> + Content-Transfer-Encoding: quoted-printable
Perhaps this is too much detail and unnecessary for the generalization
we're trying to achieve here?
Maybe the following would suffice?
The SMTP envelope and headers are passed as the 2nd argument to the
hook, exactly as they are passed to the user's Mail Transport Agent (MTA).
In effect, the email given to the user's MTA, is the contents of $2 followed
by the contents of $1.
--
Regards,
Luben
^ permalink raw reply
* Re: [PATCH v5 2/2] send-email: expose header information to git-send-email's sendemail-validate hook
From: Junio C Hamano @ 2023-01-17 4:29 UTC (permalink / raw)
To: Luben Tuikov; +Cc: Strawbridge, Michael, git@vger.kernel.org
In-Reply-To: <f31f1480-d611-f4b4-0e7b-589574943eef@amd.com>
Luben Tuikov <luben.tuikov@amd.com> writes:
> We're generally not interested in "what else" is in the SMTP envelope
> and headers.
> ...
> The idea is that hook writers would merely be grepping for a particular
> header they're interested in--it could even be a custom header, "X-something"
> for instance, and if present, they'll check the contents of that header and
> validate the patch, or perform some other action.
I am following you thus far, but ...
> So, checking that the SMTP envelope and headers, $2, is not empty suffices
> for what this patch set implements. We leave it up to the hook writers to
> inspect the SMTP envelope and headers for their particular hook purpose.
... I am lost here. To make sure that the hook writers' grepping
for a particular contents in the file $2 does find what they are
trying to find, wouldn't we want to have a test that looks for a
"known" header that should exist in the expected output (or even
better, arrange the send-email invocation so that a custom header is
injected to the output of format-patch and grep for that "known"
header)?
Thanks.
^ permalink raw reply
* Re: [PATCH v6 1/2] send-email: refactor header generation functions
From: Luben Tuikov @ 2023-01-17 4:13 UTC (permalink / raw)
To: Strawbridge, Michael, git@vger.kernel.org; +Cc: Junio C Hamano
In-Reply-To: <20230117013932.47570-2-michael.strawbridge@amd.com>
On 2023-01-16 20:39, Strawbridge, Michael wrote:
> Split process_file and send_message into easier to use functions.
> Making SMTP header information more widely available.
"more widely" --> "widely"
>
> Cc: Luben Tuikov <luben.tuikov@amd.com>
> Cc: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Michael Strawbridge <michael.strawbridge@amd.com>
> ---
> git-send-email.perl | 49 ++++++++++++++++++++++++++++-----------------
> 1 file changed, 31 insertions(+), 18 deletions(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 5861e99a6e..810dd1f1ce 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1495,16 +1495,7 @@ sub file_name_is_absolute {
> return File::Spec::Functions::file_name_is_absolute($path);
> }
>
> -# Prepares the email, then asks the user what to do.
> -#
> -# If the user chooses to send the email, it's sent and 1 is returned.
> -# If the user chooses not to send the email, 0 is returned.
> -# If the user decides they want to make further edits, -1 is returned and the
> -# caller is expected to call send_message again after the edits are performed.
> -#
> -# If an error occurs sending the email, this just dies.
> -
> -sub send_message {
> +sub gen_header {
> my @recipients = unique_email_list(@to);
> @cc = (grep { my $cc = extract_valid_address_or_die($_);
> not grep { $cc eq $_ || $_ =~ /<\Q${cc}\E>$/ } @recipients
> @@ -1546,6 +1537,22 @@ sub send_message {
> if (@xh) {
> $header .= join("\n", @xh) . "\n";
> }
> + my $recipients_ref = \@recipients;
> + return ($recipients_ref, $to, $date, $gitversion, $cc, $ccline, $header);
> +}
> +
> +# Prepares the email, then asks the user what to do.
> +#
> +# If the user chooses to send the email, it's sent and 1 is returned.
> +# If the user chooses not to send the email, 0 is returned.
> +# If the user decides they want to make further edits, -1 is returned and the
> +# caller is expected to call send_message again after the edits are performed.
> +#
> +# If an error occurs sending the email, this just dies.
> +
> +sub send_message {
> + my ($recipients_ref, $to, $date, $gitversion, $cc, $ccline, $header) = gen_header();
> + my @recipients = @$recipients_ref;
>
> my @sendmail_parameters = ('-i', @recipients);
> my $raw_from = $sender;
> @@ -1735,11 +1742,8 @@ sub send_message {
> $references = $initial_in_reply_to || '';
> $message_num = 0;
>
> -# Prepares the email, prompts the user, sends it out
> -# Returns 0 if an edit was done and the function should be called again, or 1
> -# otherwise.
> -sub process_file {
> - my ($t) = @_;
> +sub pre_process_file {
> + my ($t, $quiet) = @_;
>
> open my $fh, "<", $t or die sprintf(__("can't open file %s"), $t);
>
> @@ -1893,9 +1897,9 @@ sub process_file {
> }
> close $fh;
>
> - push @to, recipients_cmd("to-cmd", "to", $to_cmd, $t)
> + push @to, recipients_cmd("to-cmd", "to", $to_cmd, $t, $quiet)
> if defined $to_cmd;
> - push @cc, recipients_cmd("cc-cmd", "cc", $cc_cmd, $t)
> + push @cc, recipients_cmd("cc-cmd", "cc", $cc_cmd, $t, $quiet)
> if defined $cc_cmd && !$suppress_cc{'cccmd'};
>
> if ($broken_encoding{$t} && !$has_content_type) {
> @@ -1954,6 +1958,15 @@ sub process_file {
> @initial_to = @to;
> }
> }
> +}
> +
> +# Prepares the email, prompts the user, sends it out
Perhaps add an "and" as "... user, and sends it out."
> +# Returns 0 if an edit was done and the function should be called again, or 1
> +# otherwise.
"otherwise" is usually used on error. Perhaps we want to say here
"or 1 on the email being successfully sent out."?
--
Regards,
Luben
^ permalink raw reply
* Re: [PATCH v5 2/2] send-email: expose header information to git-send-email's sendemail-validate hook
From: Luben Tuikov @ 2023-01-17 4:09 UTC (permalink / raw)
To: Junio C Hamano, Strawbridge, Michael; +Cc: git@vger.kernel.org
In-Reply-To: <xmqqfsccii86.fsf@gitster.g>
On 2023-01-14 22:34, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> "Strawbridge, Michael" <Michael.Strawbridge@amd.com> writes:
>>>
>>>> +test_expect_success $PREREQ "--validate hook supports header argument" '
>>>> + test_when_finished "rm my-hooks.ran" &&
>>>> + write_script my-hooks/sendemail-validate <<-\EOF &&
>>>> + filesize=$(stat -c%s "$2")
>>>
>>> That "stat -c" is a GNU-ism, I think. macOS CI jobs at GitHub do
>>> not seem to like it.
>>>
>>>> + if [ "$filesize" != "0" ]; then
>>>
>>> Also, please see Documentation/CodingGuidelines to learn the subset
>>> of shell script syntax and style we adopted for this project.
>
> I'll tentatively queue this as a fix-up on top of the topic, but is
> this testing the right thing? Should we inspect "$2" and verify
> that it gives us what we expect, not just it being non-empty?
Hi Junio,
Thanks for reviewing this patch set.
We're generally not interested in "what else" is in the SMTP envelope
and headers.
The extension this patch set provides is that if a hook-writer is
interested in some SMTP header, or the contents of that header, then
there is a way to provide the SMTP envelope and thus check the headers.
Currently, $1, is identical to git-format-patch's output, (for which there
are other hooks to check that output.) This was a bit disappointing, as it
is a git-send-email hook after all, and we're interested in the "email" part
of this Git command and hook.
The idea is that hook writers would merely be grepping for a particular
header they're interested in--it could even be a custom header, "X-something"
for instance, and if present, they'll check the contents of that header and
validate the patch, or perform some other action.
So, checking that the SMTP envelope and headers, $2, is not empty suffices
for what this patch set implements. We leave it up to the hook writers to
inspect the SMTP envelope and headers for their particular hook purpose.
--
Regards,
Luben
^ permalink raw reply
* Re: [PATCH v6 1/2] send-email: refactor header generation functions
From: Luben Tuikov @ 2023-01-17 3:38 UTC (permalink / raw)
To: Strawbridge, Michael, git@vger.kernel.org; +Cc: Junio C Hamano
In-Reply-To: <20230117013932.47570-2-michael.strawbridge@amd.com>
Acked by: Luben Tuikov <luben.tuikov@amd.com>
Regards,
Luben
On 2023-01-16 20:39, Strawbridge, Michael wrote:
> Split process_file and send_message into easier to use functions.
> Making SMTP header information more widely available.
>
> Cc: Luben Tuikov <luben.tuikov@amd.com>
> Cc: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Michael Strawbridge <michael.strawbridge@amd.com>
> ---
> git-send-email.perl | 49 ++++++++++++++++++++++++++++-----------------
> 1 file changed, 31 insertions(+), 18 deletions(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 5861e99a6e..810dd1f1ce 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1495,16 +1495,7 @@ sub file_name_is_absolute {
> return File::Spec::Functions::file_name_is_absolute($path);
> }
>
> -# Prepares the email, then asks the user what to do.
> -#
> -# If the user chooses to send the email, it's sent and 1 is returned.
> -# If the user chooses not to send the email, 0 is returned.
> -# If the user decides they want to make further edits, -1 is returned and the
> -# caller is expected to call send_message again after the edits are performed.
> -#
> -# If an error occurs sending the email, this just dies.
> -
> -sub send_message {
> +sub gen_header {
> my @recipients = unique_email_list(@to);
> @cc = (grep { my $cc = extract_valid_address_or_die($_);
> not grep { $cc eq $_ || $_ =~ /<\Q${cc}\E>$/ } @recipients
> @@ -1546,6 +1537,22 @@ sub send_message {
> if (@xh) {
> $header .= join("\n", @xh) . "\n";
> }
> + my $recipients_ref = \@recipients;
> + return ($recipients_ref, $to, $date, $gitversion, $cc, $ccline, $header);
> +}
> +
> +# Prepares the email, then asks the user what to do.
> +#
> +# If the user chooses to send the email, it's sent and 1 is returned.
> +# If the user chooses not to send the email, 0 is returned.
> +# If the user decides they want to make further edits, -1 is returned and the
> +# caller is expected to call send_message again after the edits are performed.
> +#
> +# If an error occurs sending the email, this just dies.
> +
> +sub send_message {
> + my ($recipients_ref, $to, $date, $gitversion, $cc, $ccline, $header) = gen_header();
> + my @recipients = @$recipients_ref;
>
> my @sendmail_parameters = ('-i', @recipients);
> my $raw_from = $sender;
> @@ -1735,11 +1742,8 @@ sub send_message {
> $references = $initial_in_reply_to || '';
> $message_num = 0;
>
> -# Prepares the email, prompts the user, sends it out
> -# Returns 0 if an edit was done and the function should be called again, or 1
> -# otherwise.
> -sub process_file {
> - my ($t) = @_;
> +sub pre_process_file {
> + my ($t, $quiet) = @_;
>
> open my $fh, "<", $t or die sprintf(__("can't open file %s"), $t);
>
> @@ -1893,9 +1897,9 @@ sub process_file {
> }
> close $fh;
>
> - push @to, recipients_cmd("to-cmd", "to", $to_cmd, $t)
> + push @to, recipients_cmd("to-cmd", "to", $to_cmd, $t, $quiet)
> if defined $to_cmd;
> - push @cc, recipients_cmd("cc-cmd", "cc", $cc_cmd, $t)
> + push @cc, recipients_cmd("cc-cmd", "cc", $cc_cmd, $t, $quiet)
> if defined $cc_cmd && !$suppress_cc{'cccmd'};
>
> if ($broken_encoding{$t} && !$has_content_type) {
> @@ -1954,6 +1958,15 @@ sub process_file {
> @initial_to = @to;
> }
> }
> +}
> +
> +# Prepares the email, prompts the user, sends it out
> +# Returns 0 if an edit was done and the function should be called again, or 1
> +# otherwise.
> +sub process_file {
> + my ($t) = @_;
> +
> + pre_process_file($t, $quiet);
>
> my $message_was_sent = send_message();
> if ($message_was_sent == -1) {
> @@ -2002,7 +2015,7 @@ sub process_file {
> # Execute a command (e.g. $to_cmd) to get a list of email addresses
> # and return a results array
> sub recipients_cmd {
> - my ($prefix, $what, $cmd, $file) = @_;
> + my ($prefix, $what, $cmd, $file, $quiet) = @_;
>
> my @addresses = ();
> open my $fh, "-|", "$cmd \Q$file\E"
^ permalink raw reply
* [PATCH v2 3/3] http: support CURLOPT_PROTOCOLS_STR
From: Jeff King @ 2023-01-17 3:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ævar Arnfjörð Bjarmason, git, Ramsay Jones
In-Reply-To: <Y8YP+R/hyNr6sEFA@coredump.intra.peff.net>
The CURLOPT_PROTOCOLS (and matching CURLOPT_REDIR_PROTOCOLS) flag was
deprecated in curl 7.85.0, and using it generate compiler warnings as of
curl 7.87.0. The path forward is to use CURLOPT_PROTOCOLS_STR, but we
can't just do so unilaterally, as it was only introduced less than a
year ago in 7.85.0.
Until that version becomes ubiquitous, we have to either disable the
deprecation warning or conditionally use the "STR" variant on newer
versions of libcurl. This patch switches to the new variant, which is
nice for two reasons:
- we don't have to worry that silencing curl's deprecation warnings
might cause us to miss other more useful ones
- we'd eventually want to move to the new variant anyway, so this gets
us set up (albeit with some extra ugly boilerplate for the
conditional)
There are a lot of ways to split up the two cases. One way would be to
abstract the storage type (strbuf versus a long), how to append
(strbuf_addstr vs bitwise OR), how to initialize, which CURLOPT to use,
and so on. But the resulting code looks pretty magical:
GIT_CURL_PROTOCOL_TYPE allowed = GIT_CURL_PROTOCOL_TYPE_INIT;
if (...http is allowed...)
GIT_CURL_PROTOCOL_APPEND(&allowed, "http", CURLOPT_HTTP);
and you end up with more "#define GIT_CURL_PROTOCOL_TYPE" macros than
actual code.
On the other end of the spectrum, we could just implement two separate
functions, one that handles a string list and one that handles bits. But
then we end up repeating our list of protocols (http, https, ftp, ftp).
This patch takes the middle ground. The run-time code is always there to
handle both types, and we just choose which one to feed to curl.
Signed-off-by: Jeff King <peff@peff.net>
---
git-curl-compat.h | 8 +++++++
http.c | 59 ++++++++++++++++++++++++++++++++++++-----------
2 files changed, 54 insertions(+), 13 deletions(-)
diff --git a/git-curl-compat.h b/git-curl-compat.h
index 56a83b6bbd..fd96b3cdff 100644
--- a/git-curl-compat.h
+++ b/git-curl-compat.h
@@ -126,4 +126,12 @@
#define GIT_CURL_HAVE_CURLSSLSET_NO_BACKENDS
#endif
+/**
+ * CURLOPT_PROTOCOLS_STR and CURLOPT_REDIR_PROTOCOLS_STR were added in 7.85.0,
+ * released in August 2022.
+ */
+#if LIBCURL_VERSION_NUM >= 0x075500
+#define GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR 1
+#endif
+
#endif
diff --git a/http.c b/http.c
index ca0fe80ddb..c4b6ddef28 100644
--- a/http.c
+++ b/http.c
@@ -764,20 +764,37 @@ void setup_curl_trace(CURL *handle)
curl_easy_setopt(handle, CURLOPT_DEBUGDATA, NULL);
}
-static long get_curl_allowed_protocols(int from_user)
+static void proto_list_append(struct strbuf *list, const char *proto)
{
- long allowed_protocols = 0;
+ if (!list)
+ return;
+ if (list->len)
+ strbuf_addch(list, ',');
+ strbuf_addstr(list, proto);
+}
- if (is_transport_allowed("http", from_user))
- allowed_protocols |= CURLPROTO_HTTP;
- if (is_transport_allowed("https", from_user))
- allowed_protocols |= CURLPROTO_HTTPS;
- if (is_transport_allowed("ftp", from_user))
- allowed_protocols |= CURLPROTO_FTP;
- if (is_transport_allowed("ftps", from_user))
- allowed_protocols |= CURLPROTO_FTPS;
+static long get_curl_allowed_protocols(int from_user, struct strbuf *list)
+{
+ long bits = 0;
- return allowed_protocols;
+ if (is_transport_allowed("http", from_user)) {
+ bits |= CURLPROTO_HTTP;
+ proto_list_append(list, "http");
+ }
+ if (is_transport_allowed("https", from_user)) {
+ bits |= CURLPROTO_HTTPS;
+ proto_list_append(list, "https");
+ }
+ if (is_transport_allowed("ftp", from_user)) {
+ bits |= CURLPROTO_FTP;
+ proto_list_append(list, "ftp");
+ }
+ if (is_transport_allowed("ftps", from_user)) {
+ bits |= CURLPROTO_FTPS;
+ proto_list_append(list, "ftps");
+ }
+
+ return bits;
}
#ifdef GIT_CURL_HAVE_CURL_HTTP_VERSION_2
@@ -921,10 +938,26 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_MAXREDIRS, 20);
curl_easy_setopt(result, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL);
+
+#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
+ {
+ struct strbuf buf = STRBUF_INIT;
+
+ get_curl_allowed_protocols(0, &buf);
+ curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS_STR, buf.buf);
+ strbuf_reset(&buf);
+
+ get_curl_allowed_protocols(-1, &buf);
+ curl_easy_setopt(result, CURLOPT_PROTOCOLS_STR, buf.buf);
+ strbuf_release(&buf);
+ }
+#else
curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS,
- get_curl_allowed_protocols(0));
+ get_curl_allowed_protocols(0, NULL));
curl_easy_setopt(result, CURLOPT_PROTOCOLS,
- get_curl_allowed_protocols(-1));
+ get_curl_allowed_protocols(-1, NULL));
+#endif
+
if (getenv("GIT_CURL_VERBOSE"))
http_trace_curl_no_data();
setup_curl_trace(result);
--
2.39.0.513.g00e40dbe01
^ permalink raw reply related
* [PATCH v2 2/3] http: prefer CURLOPT_SEEKFUNCTION to CURLOPT_IOCTLFUNCTION
From: Jeff King @ 2023-01-17 3:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ævar Arnfjörð Bjarmason, git, Ramsay Jones
In-Reply-To: <Y8YP+R/hyNr6sEFA@coredump.intra.peff.net>
The IOCTLFUNCTION option has been deprecated, and generates a compiler
warning in recent versions of curl. We can switch to using SEEKFUNCTION
instead. It was added in 2008 via curl 7.18.0; our INSTALL file already
indicates we require at least curl 7.19.4.
But there's one catch: curl says we should use CURL_SEEKFUNC_{OK,FAIL},
and those didn't arrive until 7.19.5. One workaround would be to use a
bare 0/1 here (or define our own macros). But let's just bump the
minimum required version to 7.19.5. That version is only a minor version
bump from our existing requirement, and is only a 2 month time bump for
versions that are almost 13 years old. So it's not likely that anybody
cares about the distinction.
Switching means we have to rewrite the ioctl functions into seek
functions. In some ways they are simpler (seeking is the only
operation), but in some ways more complex (the ioctl allowed only a full
rewind, but now we can seek to arbitrary offsets).
Curl will only ever use SEEK_SET (per their documentation), so I didn't
bother implementing anything else, since it would naturally be
completely untested. This seems unlikely to change, but I added an
assertion just in case.
Likewise, I doubt curl will ever try to seek outside of the buffer sizes
we've told it, but I erred on the defensive side here, rather than do an
out-of-bounds read.
Signed-off-by: Jeff King <peff@peff.net>
---
INSTALL | 2 +-
http-push.c | 4 ++--
http.c | 20 +++++++++-----------
http.h | 2 +-
remote-curl.c | 28 +++++++++++++---------------
5 files changed, 26 insertions(+), 30 deletions(-)
diff --git a/INSTALL b/INSTALL
index 3344788397..d5694f8c47 100644
--- a/INSTALL
+++ b/INSTALL
@@ -139,7 +139,7 @@ Issues of note:
not need that functionality, use NO_CURL to build without
it.
- Git requires version "7.19.4" or later of "libcurl" to build
+ Git requires version "7.19.5" or later of "libcurl" to build
without NO_CURL. This version requirement may be bumped in
the future.
diff --git a/http-push.c b/http-push.c
index 1b18e775d0..7f71316456 100644
--- a/http-push.c
+++ b/http-push.c
@@ -203,8 +203,8 @@ static void curl_setup_http(CURL *curl, const char *url,
curl_easy_setopt(curl, CURLOPT_INFILE, buffer);
curl_easy_setopt(curl, CURLOPT_INFILESIZE, buffer->buf.len);
curl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);
- curl_easy_setopt(curl, CURLOPT_IOCTLFUNCTION, ioctl_buffer);
- curl_easy_setopt(curl, CURLOPT_IOCTLDATA, buffer);
+ curl_easy_setopt(curl, CURLOPT_SEEKFUNCTION, seek_buffer);
+ curl_easy_setopt(curl, CURLOPT_SEEKDATA, buffer);
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_fn);
curl_easy_setopt(curl, CURLOPT_NOBODY, 0);
curl_easy_setopt(curl, CURLOPT_CUSTOMREQUEST, custom_req);
diff --git a/http.c b/http.c
index 8a5ba3f477..ca0fe80ddb 100644
--- a/http.c
+++ b/http.c
@@ -157,21 +157,19 @@ size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
return size / eltsize;
}
-curlioerr ioctl_buffer(CURL *handle, int cmd, void *clientp)
+int seek_buffer(void *clientp, curl_off_t offset, int origin)
{
struct buffer *buffer = clientp;
- switch (cmd) {
- case CURLIOCMD_NOP:
- return CURLIOE_OK;
-
- case CURLIOCMD_RESTARTREAD:
- buffer->posn = 0;
- return CURLIOE_OK;
-
- default:
- return CURLIOE_UNKNOWNCMD;
+ if (origin != SEEK_SET)
+ BUG("seek_buffer only handles SEEK_SET");
+ if (offset < 0 || offset >= buffer->buf.len) {
+ error("curl seek would be outside of buffer");
+ return CURL_SEEKFUNC_FAIL;
}
+
+ buffer->posn = offset;
+ return CURL_SEEKFUNC_OK;
}
size_t fwrite_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
diff --git a/http.h b/http.h
index 3c94c47910..77c042706c 100644
--- a/http.h
+++ b/http.h
@@ -40,7 +40,7 @@ struct buffer {
size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *strbuf);
size_t fwrite_buffer(char *ptr, size_t eltsize, size_t nmemb, void *strbuf);
size_t fwrite_null(char *ptr, size_t eltsize, size_t nmemb, void *strbuf);
-curlioerr ioctl_buffer(CURL *handle, int cmd, void *clientp);
+int seek_buffer(void *clientp, curl_off_t offset, int origin);
/* Slot lifecycle functions */
struct active_request_slot *get_active_slot(void);
diff --git a/remote-curl.c b/remote-curl.c
index 72dfb8fb86..a76b6405eb 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -717,25 +717,23 @@ static size_t rpc_out(void *ptr, size_t eltsize,
return avail;
}
-static curlioerr rpc_ioctl(CURL *handle, int cmd, void *clientp)
+static int rpc_seek(void *clientp, curl_off_t offset, int origin)
{
struct rpc_state *rpc = clientp;
- switch (cmd) {
- case CURLIOCMD_NOP:
- return CURLIOE_OK;
+ if (origin != SEEK_SET)
+ BUG("rpc_seek only handles SEEK_SET, not %d", origin);
- case CURLIOCMD_RESTARTREAD:
- if (rpc->initial_buffer) {
- rpc->pos = 0;
- return CURLIOE_OK;
+ if (rpc->initial_buffer) {
+ if (offset < 0 || offset > rpc->len) {
+ error("curl seek would be outside of rpc buffer");
+ return CURL_SEEKFUNC_FAIL;
}
- error(_("unable to rewind rpc post data - try increasing http.postBuffer"));
- return CURLIOE_FAILRESTART;
-
- default:
- return CURLIOE_UNKNOWNCMD;
+ rpc->pos = offset;
+ return CURL_SEEKFUNC_OK;
}
+ error(_("unable to rewind rpc post data - try increasing http.postBuffer"));
+ return CURL_SEEKFUNC_FAIL;
}
struct check_pktline_state {
@@ -959,8 +957,8 @@ static int post_rpc(struct rpc_state *rpc, int stateless_connect, int flush_rece
rpc->initial_buffer = 1;
curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, rpc_out);
curl_easy_setopt(slot->curl, CURLOPT_INFILE, rpc);
- curl_easy_setopt(slot->curl, CURLOPT_IOCTLFUNCTION, rpc_ioctl);
- curl_easy_setopt(slot->curl, CURLOPT_IOCTLDATA, rpc);
+ curl_easy_setopt(slot->curl, CURLOPT_SEEKFUNCTION, rpc_seek);
+ curl_easy_setopt(slot->curl, CURLOPT_SEEKDATA, rpc);
if (options.verbosity > 1) {
fprintf(stderr, "POST %s (chunked)\n", rpc->service_name);
fflush(stderr);
--
2.39.0.513.g00e40dbe01
^ permalink raw reply related
* [PATCH v2 1/3] http-push: prefer CURLOPT_UPLOAD to CURLOPT_PUT
From: Jeff King @ 2023-01-17 3:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ævar Arnfjörð Bjarmason, git, Ramsay Jones
In-Reply-To: <Y8YP+R/hyNr6sEFA@coredump.intra.peff.net>
The two options do exactly the same thing, but the latter has been
deprecated and in recent versions of curl may produce a compiler
warning. Since the UPLOAD form is available everywhere (it was
introduced in the year 2000 by curl 7.1), we can just switch to it.
Signed-off-by: Jeff King <peff@peff.net>
---
http-push.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/http-push.c b/http-push.c
index 5f4340a36e..1b18e775d0 100644
--- a/http-push.c
+++ b/http-push.c
@@ -198,7 +198,7 @@ static void curl_setup_http(CURL *curl, const char *url,
const char *custom_req, struct buffer *buffer,
curl_write_callback write_fn)
{
- curl_easy_setopt(curl, CURLOPT_PUT, 1);
+ curl_easy_setopt(curl, CURLOPT_UPLOAD, 1);
curl_easy_setopt(curl, CURLOPT_URL, url);
curl_easy_setopt(curl, CURLOPT_INFILE, buffer);
curl_easy_setopt(curl, CURLOPT_INFILESIZE, buffer->buf.len);
--
2.39.0.513.g00e40dbe01
^ permalink raw reply related
* Re: [PATCH 1/3] http-push: prefer CURLOPT_UPLOAD to CURLOPT_PUT
From: Jeff King @ 2023-01-17 3:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Ramsay Jones
In-Reply-To: <Y8YQBbjjf+i8BWL6@coredump.intra.peff.net>
On Mon, Jan 16, 2023 at 10:03:33PM -0500, Jeff King wrote:
> The two options do exactly the same thing, but the latter has been
> deprecated and in recent versions of curl may produce a compiler
> warning. Since the UPLOAD form is available everywhere (it was
> introduced in the year 2000 by curl 7.1), we can just switch to it.
>
> Signed-off-by: Jeff King <peff@peff.net>
Whoops, this was supposed to be part of v2, but I accidentally sent it
in reply to the wrong part of the thread. Please ignore, and I'll resend
in the right spot.
-Peff
^ permalink raw reply
* [PATCH 1/3] http-push: prefer CURLOPT_UPLOAD to CURLOPT_PUT
From: Jeff King @ 2023-01-17 3:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Ramsay Jones
In-Reply-To: <Y8Ljp7yEWTUd92w8@coredump.intra.peff.net>
The two options do exactly the same thing, but the latter has been
deprecated and in recent versions of curl may produce a compiler
warning. Since the UPLOAD form is available everywhere (it was
introduced in the year 2000 by curl 7.1), we can just switch to it.
Signed-off-by: Jeff King <peff@peff.net>
---
http-push.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/http-push.c b/http-push.c
index 5f4340a36e..1b18e775d0 100644
--- a/http-push.c
+++ b/http-push.c
@@ -198,7 +198,7 @@ static void curl_setup_http(CURL *curl, const char *url,
const char *custom_req, struct buffer *buffer,
curl_write_callback write_fn)
{
- curl_easy_setopt(curl, CURLOPT_PUT, 1);
+ curl_easy_setopt(curl, CURLOPT_UPLOAD, 1);
curl_easy_setopt(curl, CURLOPT_URL, url);
curl_easy_setopt(curl, CURLOPT_INFILE, buffer);
curl_easy_setopt(curl, CURLOPT_INFILESIZE, buffer->buf.len);
--
2.39.0.513.g00e40dbe01
^ permalink raw reply related
* [PATCH v2] avoiding deprecated curl options
From: Jeff King @ 2023-01-17 3:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ævar Arnfjörð Bjarmason, git, Ramsay Jones
In-Reply-To: <Y8RddcM9Vr71ljp4@coredump.intra.peff.net>
On Sun, Jan 15, 2023 at 03:09:26PM -0500, Jeff King wrote:
> So I took a look at just dropping the deprecated bits, and it wasn't
> _too_ bad. Here's that series. The first two I hope are obviously good.
> The third one is _ugly_, but at least it punts on the whole "how should
> we silence this" argument, and it takes us in the direction we'd
> ultimately want to go.
>
> [1/3]: http-push: prefer CURLOPT_UPLOAD to CURLOPT_PUT
> [2/3]: http: prefer CURLOPT_SEEKFUNCTION to CURLOPT_IOCTLFUNCTION
> [3/3]: http: support CURLOPT_PROTOCOLS_STR
In the interests of wrapping this up, here's a v2 that:
- bumps the required curl version to 7.19.5 in patch 2
- aims for slightly better readability in the final code of patch 3,
versus minimizing the diff
As discussed, there are a lot of different ways one could do patch 3,
but I really don't think it's worth spending that much brain power on.
What's here is correct (most important), and I think should be easy to
clean up when we can eventually drop the old style.
[1/3]: http-push: prefer CURLOPT_UPLOAD to CURLOPT_PUT
[2/3]: http: prefer CURLOPT_SEEKFUNCTION to CURLOPT_IOCTLFUNCTION
[3/3]: http: support CURLOPT_PROTOCOLS_STR
INSTALL | 2 +-
git-curl-compat.h | 8 +++++
http-push.c | 6 ++--
http.c | 79 +++++++++++++++++++++++++++++++++--------------
http.h | 2 +-
remote-curl.c | 28 ++++++++---------
6 files changed, 81 insertions(+), 44 deletions(-)
1: 2229c0468f = 1: 5ae6831af5 http-push: prefer CURLOPT_UPLOAD to CURLOPT_PUT
2: 00120fa40e ! 2: 5be76d74de http: prefer CURLOPT_SEEKFUNCTION to CURLOPT_IOCTLFUNCTION
@@ Commit message
instead. It was added in 2008 via curl 7.18.0; our INSTALL file already
indicates we require at least curl 7.19.4.
- We have to rewrite the ioctl functions into seek functions. In some ways
- they are simpler (seeking is the only operation), but in some ways more
- complex (the ioctl allowed only a full rewind, but now we can seek to
- arbitrary offsets).
+ But there's one catch: curl says we should use CURL_SEEKFUNC_{OK,FAIL},
+ and those didn't arrive until 7.19.5. One workaround would be to use a
+ bare 0/1 here (or define our own macros). But let's just bump the
+ minimum required version to 7.19.5. That version is only a minor version
+ bump from our existing requirement, and is only a 2 month time bump for
+ versions that are almost 13 years old. So it's not likely that anybody
+ cares about the distinction.
+
+ Switching means we have to rewrite the ioctl functions into seek
+ functions. In some ways they are simpler (seeking is the only
+ operation), but in some ways more complex (the ioctl allowed only a full
+ rewind, but now we can seek to arbitrary offsets).
Curl will only ever use SEEK_SET (per their documentation), so I didn't
bother implementing anything else, since it would naturally be
@@ Commit message
Signed-off-by: Jeff King <peff@peff.net>
+ ## INSTALL ##
+@@ INSTALL: Issues of note:
+ not need that functionality, use NO_CURL to build without
+ it.
+
+- Git requires version "7.19.4" or later of "libcurl" to build
++ Git requires version "7.19.5" or later of "libcurl" to build
+ without NO_CURL. This version requirement may be bumped in
+ the future.
+
+
## http-push.c ##
@@ http-push.c: static void curl_setup_http(CURL *curl, const char *url,
curl_easy_setopt(curl, CURLOPT_INFILE, buffer);
3: 22eb2fd0fe ! 3: d2c28e22e1 http: support CURLOPT_PROTOCOLS_STR
@@ http.c: void setup_curl_trace(CURL *handle)
}
-static long get_curl_allowed_protocols(int from_user)
-+static void proto_list_append(struct strbuf *list_str, const char *proto_str,
-+ long *list_bits, long proto_bits)
-+{
-+ *list_bits |= proto_bits;
-+ if (list_str) {
-+ if (list_str->len)
-+ strbuf_addch(list_str, ',');
-+ strbuf_addstr(list_str, proto_str);
-+ }
-+}
-+
-+static long get_curl_allowed_protocols(int from_user, struct strbuf *list)
++static void proto_list_append(struct strbuf *list, const char *proto)
{
- long allowed_protocols = 0;
+- long allowed_protocols = 0;
++ if (!list)
++ return;
++ if (list->len)
++ strbuf_addch(list, ',');
++ strbuf_addstr(list, proto);
++}
- if (is_transport_allowed("http", from_user))
+- if (is_transport_allowed("http", from_user))
- allowed_protocols |= CURLPROTO_HTTP;
-+ proto_list_append(list, "http", &allowed_protocols, CURLPROTO_HTTP);
- if (is_transport_allowed("https", from_user))
+- if (is_transport_allowed("https", from_user))
- allowed_protocols |= CURLPROTO_HTTPS;
-+ proto_list_append(list, "https", &allowed_protocols, CURLPROTO_HTTPS);
- if (is_transport_allowed("ftp", from_user))
+- if (is_transport_allowed("ftp", from_user))
- allowed_protocols |= CURLPROTO_FTP;
-+ proto_list_append(list, "ftp", &allowed_protocols, CURLPROTO_FTP);
- if (is_transport_allowed("ftps", from_user))
+- if (is_transport_allowed("ftps", from_user))
- allowed_protocols |= CURLPROTO_FTPS;
-+ proto_list_append(list, "ftps", &allowed_protocols, CURLPROTO_FTPS);
++static long get_curl_allowed_protocols(int from_user, struct strbuf *list)
++{
++ long bits = 0;
- return allowed_protocols;
+- return allowed_protocols;
++ if (is_transport_allowed("http", from_user)) {
++ bits |= CURLPROTO_HTTP;
++ proto_list_append(list, "http");
++ }
++ if (is_transport_allowed("https", from_user)) {
++ bits |= CURLPROTO_HTTPS;
++ proto_list_append(list, "https");
++ }
++ if (is_transport_allowed("ftp", from_user)) {
++ bits |= CURLPROTO_FTP;
++ proto_list_append(list, "ftp");
++ }
++ if (is_transport_allowed("ftps", from_user)) {
++ bits |= CURLPROTO_FTPS;
++ proto_list_append(list, "ftps");
++ }
++
++ return bits;
}
+
+ #ifdef GIT_CURL_HAVE_CURL_HTTP_VERSION_2
@@ http.c: static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_MAXREDIRS, 20);
^ permalink raw reply
* [PATCH v5 0/2] send-email: expose header information to git-send-email's sendemail-validate hook
From: Strawbridge, Michael @ 2023-01-17 1:49 UTC (permalink / raw)
To: git@vger.kernel.org; +Cc: Strawbridge, Michael
In-Reply-To: <20230110211452.2568535-1-michael.strawbridge@amd.com>
Please ignore this thread.
Michael Strawbridge
--
2.34.1
^ permalink raw reply
* [PATCH v6 2/2] send-email: expose header information to git-send-email's sendemail-validate hook
From: Strawbridge, Michael @ 2023-01-17 1:39 UTC (permalink / raw)
To: git@vger.kernel.org; +Cc: Strawbridge, Michael, Tuikov, Luben, Junio C Hamano
In-Reply-To: <20230117013932.47570-1-michael.strawbridge@amd.com>
To allow further flexibility in the git hook, the SMTP header
information of the email that git-send-email intends to send, is now
passed as a 2nd argument to the sendemail-validate hook.
As an example, this can be useful for acting upon keywords in the
subject or specific email addresses.
Cc: Luben Tuikov <luben.tuikov@amd.com>
Cc: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Michael Strawbridge <michael.strawbridge@amd.com>
---
Documentation/githooks.txt | 29 +++++++++++++++++++----
git-send-email.perl | 31 +++++++++++++++++--------
t/t9001-send-email.sh | 47 ++++++++++++++++++++++++++++++++++++--
3 files changed, 91 insertions(+), 16 deletions(-)
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index a16e62bc8c..e80f481efd 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -583,10 +583,31 @@ processed by rebase.
sendemail-validate
~~~~~~~~~~~~~~~~~~
-This hook is invoked by linkgit:git-send-email[1]. It takes a single parameter,
-the name of the file that holds the e-mail to be sent. Exiting with a
-non-zero status causes `git send-email` to abort before sending any
-e-mails.
+This hook is invoked by linkgit:git-send-email[1].
+
+It takes these command line arguments:
+1. the name of the file that holds the e-mail to be sent.
+2. the name of the file that holds the SMTP headers to be used.
+
+The SMTP headers will be passed to the hook in the below format.
+Take notice of the capitalization and multi-line tab structure.
+
+ From: Example <from@example.com>
+ To: to@example.com
+ Cc: cc@example.com,
+ A <author@example.com>,
+ One <one@example.com>,
+ two@example.com
+ Subject: PATCH-STRING
+ Date: DATE-STRING
+ Message-Id: MESSAGE-ID-STRING
+ X-Mailer: X-MAILER-STRING
+ Reply-To: Reply <reply@example.com>
+ MIME-Version: 1.0
+ Content-Transfer-Encoding: quoted-printable
+
+Exiting with a non-zero status causes `git send-email` to abort
+before sending any e-mails.
fsmonitor-watchman
~~~~~~~~~~~~~~~~~~
diff --git a/git-send-email.perl b/git-send-email.perl
index 810dd1f1ce..b2adca515e 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -787,14 +787,6 @@ sub is_format_patch_arg {
@files = handle_backup_files(@files);
-if ($validate) {
- foreach my $f (@files) {
- unless (-p $f) {
- validate_patch($f, $target_xfer_encoding);
- }
- }
-}
-
if (@files) {
unless ($quiet) {
print $_,"\n" for (@files);
@@ -1738,6 +1730,16 @@ sub send_message {
return 1;
}
+if ($validate) {
+ foreach my $f (@files) {
+ unless (-p $f) {
+ pre_process_file($f, 1);
+
+ validate_patch($f, $target_xfer_encoding);
+ }
+ }
+}
+
$in_reply_to = $initial_in_reply_to;
$references = $initial_in_reply_to || '';
$message_num = 0;
@@ -2101,11 +2103,20 @@ sub validate_patch {
chdir($repo->wc_path() or $repo->repo_path())
or die("chdir: $!");
local $ENV{"GIT_DIR"} = $repo->repo_path();
+
+ my ($recipients_ref, $to, $date, $gitversion, $cc, $ccline, $header) = gen_header();
+
+ require File::Temp;
+ my ($header_filehandle, $header_filename) = File::Temp::tempfile(
+ ".gitsendemail.header.XXXXXX", DIR => $repo->repo_path());
+ print $header_filehandle $header;
+
my @cmd = ("git", "hook", "run", "--ignore-missing",
$hook_name, "--");
- my @cmd_msg = (@cmd, "<patch>");
- my @cmd_run = (@cmd, $target);
+ my @cmd_msg = (@cmd, "<patch>", "<header>");
+ my @cmd_run = (@cmd, $target, $header_filename);
$hook_error = system_or_msg(\@cmd_run, undef, "@cmd_msg");
+ unlink($header_filehandle);
chdir($cwd_save) or die("chdir: $!");
}
if ($hook_error) {
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 1130ef21b3..346ff1463e 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -540,7 +540,7 @@ test_expect_success $PREREQ "--validate respects relative core.hooksPath path" '
test_path_is_file my-hooks.ran &&
cat >expect <<-EOF &&
fatal: longline.patch: rejected by sendemail-validate hook
- fatal: command '"'"'git hook run --ignore-missing sendemail-validate -- <patch>'"'"' died with exit code 1
+ fatal: command '"'"'git hook run --ignore-missing sendemail-validate -- <patch> <header>'"'"' died with exit code 1
warning: no patches were sent
EOF
test_cmp expect actual
@@ -559,12 +559,55 @@ test_expect_success $PREREQ "--validate respects absolute core.hooksPath path" '
test_path_is_file my-hooks.ran &&
cat >expect <<-EOF &&
fatal: longline.patch: rejected by sendemail-validate hook
- fatal: command '"'"'git hook run --ignore-missing sendemail-validate -- <patch>'"'"' died with exit code 1
+ fatal: command '"'"'git hook run --ignore-missing sendemail-validate -- <patch> <header>'"'"' died with exit code 1
warning: no patches were sent
EOF
test_cmp expect actual
'
+test_expect_success $PREREQ 'setup expect' "
+cat >expected-headers <<\EOF
+From: Example <from@example.com>
+To: to@example.com
+Cc: cc@example.com,
+ A <author@example.com>,
+ One <one@example.com>,
+ two@example.com
+Subject: [PATCH 1/1] Second.
+Date: DATE-STRING
+Message-Id: MESSAGE-ID-STRING
+X-Mailer: X-MAILER-STRING
+Reply-To: Reply <reply@example.com>
+MIME-Version: 1.0
+Content-Transfer-Encoding: quoted-printable
+EOF
+"
+
+test_expect_success $PREREQ "--validate hook supports header argument" '
+ write_script my-hooks/sendemail-validate <<-\EOF &&
+ if test -s "$2"
+ then
+ cat "$2" >actual
+ exit 1
+ fi
+ EOF
+ test_config core.hooksPath "my-hooks" &&
+ test_must_fail git send-email \
+ --dry-run \
+ --suppress-cc=sob \
+ --from="Example <from@example.com>" \
+ --reply-to="Reply <reply@example.com>" \
+ --to=to@example.com \
+ --cc=cc@example.com \
+ --bcc=bcc@example.com \
+ --smtp-server="$(pwd)/fake.sendmail" \
+ --validate \
+ longline.patch &&
+ cat actual | replace_variable_fields \
+ >actual-headers &&
+ test_cmp expected-headers actual-headers
+'
+
for enc in 7bit 8bit quoted-printable base64
do
test_expect_success $PREREQ "--transfer-encoding=$enc produces correct header" '
--
2.34.1
^ permalink raw reply related
* [PATCH v6 1/2] send-email: refactor header generation functions
From: Strawbridge, Michael @ 2023-01-17 1:39 UTC (permalink / raw)
To: git@vger.kernel.org; +Cc: Strawbridge, Michael, Tuikov, Luben, Junio C Hamano
In-Reply-To: <20230117013932.47570-1-michael.strawbridge@amd.com>
Split process_file and send_message into easier to use functions.
Making SMTP header information more widely available.
Cc: Luben Tuikov <luben.tuikov@amd.com>
Cc: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Michael Strawbridge <michael.strawbridge@amd.com>
---
git-send-email.perl | 49 ++++++++++++++++++++++++++++-----------------
1 file changed, 31 insertions(+), 18 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 5861e99a6e..810dd1f1ce 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1495,16 +1495,7 @@ sub file_name_is_absolute {
return File::Spec::Functions::file_name_is_absolute($path);
}
-# Prepares the email, then asks the user what to do.
-#
-# If the user chooses to send the email, it's sent and 1 is returned.
-# If the user chooses not to send the email, 0 is returned.
-# If the user decides they want to make further edits, -1 is returned and the
-# caller is expected to call send_message again after the edits are performed.
-#
-# If an error occurs sending the email, this just dies.
-
-sub send_message {
+sub gen_header {
my @recipients = unique_email_list(@to);
@cc = (grep { my $cc = extract_valid_address_or_die($_);
not grep { $cc eq $_ || $_ =~ /<\Q${cc}\E>$/ } @recipients
@@ -1546,6 +1537,22 @@ sub send_message {
if (@xh) {
$header .= join("\n", @xh) . "\n";
}
+ my $recipients_ref = \@recipients;
+ return ($recipients_ref, $to, $date, $gitversion, $cc, $ccline, $header);
+}
+
+# Prepares the email, then asks the user what to do.
+#
+# If the user chooses to send the email, it's sent and 1 is returned.
+# If the user chooses not to send the email, 0 is returned.
+# If the user decides they want to make further edits, -1 is returned and the
+# caller is expected to call send_message again after the edits are performed.
+#
+# If an error occurs sending the email, this just dies.
+
+sub send_message {
+ my ($recipients_ref, $to, $date, $gitversion, $cc, $ccline, $header) = gen_header();
+ my @recipients = @$recipients_ref;
my @sendmail_parameters = ('-i', @recipients);
my $raw_from = $sender;
@@ -1735,11 +1742,8 @@ sub send_message {
$references = $initial_in_reply_to || '';
$message_num = 0;
-# Prepares the email, prompts the user, sends it out
-# Returns 0 if an edit was done and the function should be called again, or 1
-# otherwise.
-sub process_file {
- my ($t) = @_;
+sub pre_process_file {
+ my ($t, $quiet) = @_;
open my $fh, "<", $t or die sprintf(__("can't open file %s"), $t);
@@ -1893,9 +1897,9 @@ sub process_file {
}
close $fh;
- push @to, recipients_cmd("to-cmd", "to", $to_cmd, $t)
+ push @to, recipients_cmd("to-cmd", "to", $to_cmd, $t, $quiet)
if defined $to_cmd;
- push @cc, recipients_cmd("cc-cmd", "cc", $cc_cmd, $t)
+ push @cc, recipients_cmd("cc-cmd", "cc", $cc_cmd, $t, $quiet)
if defined $cc_cmd && !$suppress_cc{'cccmd'};
if ($broken_encoding{$t} && !$has_content_type) {
@@ -1954,6 +1958,15 @@ sub process_file {
@initial_to = @to;
}
}
+}
+
+# Prepares the email, prompts the user, sends it out
+# Returns 0 if an edit was done and the function should be called again, or 1
+# otherwise.
+sub process_file {
+ my ($t) = @_;
+
+ pre_process_file($t, $quiet);
my $message_was_sent = send_message();
if ($message_was_sent == -1) {
@@ -2002,7 +2015,7 @@ sub process_file {
# Execute a command (e.g. $to_cmd) to get a list of email addresses
# and return a results array
sub recipients_cmd {
- my ($prefix, $what, $cmd, $file) = @_;
+ my ($prefix, $what, $cmd, $file, $quiet) = @_;
my @addresses = ();
open my $fh, "-|", "$cmd \Q$file\E"
--
2.34.1
^ permalink raw reply related
* [PATCH v6 0/2] send-email: expose header information to git-send-email's sendemail-validate hook
From: Strawbridge, Michael @ 2023-01-17 1:39 UTC (permalink / raw)
To: git@vger.kernel.org; +Cc: Strawbridge, Michael
Thank you for all the great feedback! At your suggestion I improved the
test for the header argument and the documentation.
Michael Strawbridge (2):
send-email: refactor header generation functions
send-email: expose header information to git-send-email's
sendemail-validate hook
Documentation/githooks.txt | 29 ++++++++++++--
git-send-email.perl | 80 +++++++++++++++++++++++++-------------
t/t9001-send-email.sh | 47 +++++++++++++++++++++-
3 files changed, 122 insertions(+), 34 deletions(-)
--
2.34.1
^ permalink raw reply
* [PATCH v6 0/2] send-email: expose header information to git-send-email's sendemail-validate hook
From: Strawbridge, Michael @ 2023-01-17 1:37 UTC (permalink / raw)
To: git@vger.kernel.org; +Cc: Strawbridge, Michael
In-Reply-To: <20230117013709.47054-1-michael.strawbridge@amd.com>
Thank you for all the great feedback! At your suggestion I improved the
test for the header argument and the documentation.
Michael Strawbridge (2):
send-email: refactor header generation functions
send-email: expose header information to git-send-email's
sendemail-validate hook
Documentation/githooks.txt | 29 ++++++++++++--
git-send-email.perl | 80 +++++++++++++++++++++++++-------------
t/t9001-send-email.sh | 47 +++++++++++++++++++++-
3 files changed, 122 insertions(+), 34 deletions(-)
--
2.34.1
^ permalink raw reply
* [PATCH v5 2/2] send-email: expose header information to git-send-email's sendemail-validate hook
From: Strawbridge, Michael @ 2023-01-17 1:37 UTC (permalink / raw)
To: git@vger.kernel.org; +Cc: Strawbridge, Michael, Tuikov, Luben, Junio C Hamano
In-Reply-To: <20230117013709.47054-1-michael.strawbridge@amd.com>
To allow further flexibility in the git hook, the SMTP header
information of the email that git-send-email intends to send, is now
passed as a 2nd argument to the sendemail-validate hook.
As an example, this can be useful for acting upon keywords in the
subject or specific email addresses.
Cc: Luben Tuikov <luben.tuikov@amd.com>
Cc: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Michael Strawbridge <michael.strawbridge@amd.com>
---
Documentation/githooks.txt | 17 +++++++++++++----
git-send-email.perl | 31 +++++++++++++++++++++----------
t/t9001-send-email.sh | 29 +++++++++++++++++++++++++++--
3 files changed, 61 insertions(+), 16 deletions(-)
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index a16e62bc8c..2b5c6640cc 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -583,10 +583,19 @@ processed by rebase.
sendemail-validate
~~~~~~~~~~~~~~~~~~
-This hook is invoked by linkgit:git-send-email[1]. It takes a single parameter,
-the name of the file that holds the e-mail to be sent. Exiting with a
-non-zero status causes `git send-email` to abort before sending any
-e-mails.
+This hook is invoked by linkgit:git-send-email[1].
+
+It takes these command line arguments:
+1. the name of the file that holds the e-mail to be sent.
+2. the name of the file that holds the SMTP headers to be used.
+
+The hook doesn't need to support multiple header names (for example only Cc
+is passed). However, it does need to understand that lines beginning with
+whitespace belong to the previous header. The header information follows
+the same format as the confirmation given at the end of send-email.
+
+Exiting with a non-zero status causes `git send-email` to abort
+before sending any e-mails.
fsmonitor-watchman
~~~~~~~~~~~~~~~~~~
diff --git a/git-send-email.perl b/git-send-email.perl
index 810dd1f1ce..b2adca515e 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -787,14 +787,6 @@ sub is_format_patch_arg {
@files = handle_backup_files(@files);
-if ($validate) {
- foreach my $f (@files) {
- unless (-p $f) {
- validate_patch($f, $target_xfer_encoding);
- }
- }
-}
-
if (@files) {
unless ($quiet) {
print $_,"\n" for (@files);
@@ -1738,6 +1730,16 @@ sub send_message {
return 1;
}
+if ($validate) {
+ foreach my $f (@files) {
+ unless (-p $f) {
+ pre_process_file($f, 1);
+
+ validate_patch($f, $target_xfer_encoding);
+ }
+ }
+}
+
$in_reply_to = $initial_in_reply_to;
$references = $initial_in_reply_to || '';
$message_num = 0;
@@ -2101,11 +2103,20 @@ sub validate_patch {
chdir($repo->wc_path() or $repo->repo_path())
or die("chdir: $!");
local $ENV{"GIT_DIR"} = $repo->repo_path();
+
+ my ($recipients_ref, $to, $date, $gitversion, $cc, $ccline, $header) = gen_header();
+
+ require File::Temp;
+ my ($header_filehandle, $header_filename) = File::Temp::tempfile(
+ ".gitsendemail.header.XXXXXX", DIR => $repo->repo_path());
+ print $header_filehandle $header;
+
my @cmd = ("git", "hook", "run", "--ignore-missing",
$hook_name, "--");
- my @cmd_msg = (@cmd, "<patch>");
- my @cmd_run = (@cmd, $target);
+ my @cmd_msg = (@cmd, "<patch>", "<header>");
+ my @cmd_run = (@cmd, $target, $header_filename);
$hook_error = system_or_msg(\@cmd_run, undef, "@cmd_msg");
+ unlink($header_filehandle);
chdir($cwd_save) or die("chdir: $!");
}
if ($hook_error) {
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 1130ef21b3..f02b1eba16 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -540,7 +540,7 @@ test_expect_success $PREREQ "--validate respects relative core.hooksPath path" '
test_path_is_file my-hooks.ran &&
cat >expect <<-EOF &&
fatal: longline.patch: rejected by sendemail-validate hook
- fatal: command '"'"'git hook run --ignore-missing sendemail-validate -- <patch>'"'"' died with exit code 1
+ fatal: command '"'"'git hook run --ignore-missing sendemail-validate -- <patch> <header>'"'"' died with exit code 1
warning: no patches were sent
EOF
test_cmp expect actual
@@ -559,7 +559,32 @@ test_expect_success $PREREQ "--validate respects absolute core.hooksPath path" '
test_path_is_file my-hooks.ran &&
cat >expect <<-EOF &&
fatal: longline.patch: rejected by sendemail-validate hook
- fatal: command '"'"'git hook run --ignore-missing sendemail-validate -- <patch>'"'"' died with exit code 1
+ fatal: command '"'"'git hook run --ignore-missing sendemail-validate -- <patch> <header>'"'"' died with exit code 1
+ warning: no patches were sent
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success $PREREQ "--validate hook supports header argument" '
+ test_when_finished "rm my-hooks.ran" &&
+ write_script my-hooks/sendemail-validate <<-\EOF &&
+ filesize=$(stat -c%s "$2")
+ if [ "$filesize" != "0" ]; then
+ >my-hooks.ran
+ fi
+ exit 1
+ EOF
+ test_config core.hooksPath "my-hooks" &&
+ test_must_fail git send-email \
+ --from="Example <nobody@example.com>" \
+ --to=nobody@example.com \
+ --smtp-server="$(pwd)/fake.sendmail" \
+ --validate \
+ longline.patch 2>actual &&
+ test_path_is_file my-hooks.ran &&
+ cat >expect <<-EOF &&
+ fatal: longline.patch: rejected by sendemail-validate hook
+ fatal: command '"'"'git hook run --ignore-missing sendemail-validate -- <patch> <header>'"'"' died with exit code 1
warning: no patches were sent
EOF
test_cmp expect actual
--
2.34.1
^ permalink raw reply related
* [PATCH v5 1/2] send-email: refactor header generation functions
From: Strawbridge, Michael @ 2023-01-17 1:37 UTC (permalink / raw)
To: git@vger.kernel.org; +Cc: Strawbridge, Michael, Tuikov, Luben, Junio C Hamano
In-Reply-To: <20230117013709.47054-1-michael.strawbridge@amd.com>
Split process_file and send_message into easier to use functions.
Making SMTP header information more widely available.
Cc: Luben Tuikov <luben.tuikov@amd.com>
Cc: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Michael Strawbridge <michael.strawbridge@amd.com>
---
git-send-email.perl | 49 ++++++++++++++++++++++++++++-----------------
1 file changed, 31 insertions(+), 18 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 5861e99a6e..810dd1f1ce 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1495,16 +1495,7 @@ sub file_name_is_absolute {
return File::Spec::Functions::file_name_is_absolute($path);
}
-# Prepares the email, then asks the user what to do.
-#
-# If the user chooses to send the email, it's sent and 1 is returned.
-# If the user chooses not to send the email, 0 is returned.
-# If the user decides they want to make further edits, -1 is returned and the
-# caller is expected to call send_message again after the edits are performed.
-#
-# If an error occurs sending the email, this just dies.
-
-sub send_message {
+sub gen_header {
my @recipients = unique_email_list(@to);
@cc = (grep { my $cc = extract_valid_address_or_die($_);
not grep { $cc eq $_ || $_ =~ /<\Q${cc}\E>$/ } @recipients
@@ -1546,6 +1537,22 @@ sub send_message {
if (@xh) {
$header .= join("\n", @xh) . "\n";
}
+ my $recipients_ref = \@recipients;
+ return ($recipients_ref, $to, $date, $gitversion, $cc, $ccline, $header);
+}
+
+# Prepares the email, then asks the user what to do.
+#
+# If the user chooses to send the email, it's sent and 1 is returned.
+# If the user chooses not to send the email, 0 is returned.
+# If the user decides they want to make further edits, -1 is returned and the
+# caller is expected to call send_message again after the edits are performed.
+#
+# If an error occurs sending the email, this just dies.
+
+sub send_message {
+ my ($recipients_ref, $to, $date, $gitversion, $cc, $ccline, $header) = gen_header();
+ my @recipients = @$recipients_ref;
my @sendmail_parameters = ('-i', @recipients);
my $raw_from = $sender;
@@ -1735,11 +1742,8 @@ sub send_message {
$references = $initial_in_reply_to || '';
$message_num = 0;
-# Prepares the email, prompts the user, sends it out
-# Returns 0 if an edit was done and the function should be called again, or 1
-# otherwise.
-sub process_file {
- my ($t) = @_;
+sub pre_process_file {
+ my ($t, $quiet) = @_;
open my $fh, "<", $t or die sprintf(__("can't open file %s"), $t);
@@ -1893,9 +1897,9 @@ sub process_file {
}
close $fh;
- push @to, recipients_cmd("to-cmd", "to", $to_cmd, $t)
+ push @to, recipients_cmd("to-cmd", "to", $to_cmd, $t, $quiet)
if defined $to_cmd;
- push @cc, recipients_cmd("cc-cmd", "cc", $cc_cmd, $t)
+ push @cc, recipients_cmd("cc-cmd", "cc", $cc_cmd, $t, $quiet)
if defined $cc_cmd && !$suppress_cc{'cccmd'};
if ($broken_encoding{$t} && !$has_content_type) {
@@ -1954,6 +1958,15 @@ sub process_file {
@initial_to = @to;
}
}
+}
+
+# Prepares the email, prompts the user, sends it out
+# Returns 0 if an edit was done and the function should be called again, or 1
+# otherwise.
+sub process_file {
+ my ($t) = @_;
+
+ pre_process_file($t, $quiet);
my $message_was_sent = send_message();
if ($message_was_sent == -1) {
@@ -2002,7 +2015,7 @@ sub process_file {
# Execute a command (e.g. $to_cmd) to get a list of email addresses
# and return a results array
sub recipients_cmd {
- my ($prefix, $what, $cmd, $file) = @_;
+ my ($prefix, $what, $cmd, $file, $quiet) = @_;
my @addresses = ();
open my $fh, "-|", "$cmd \Q$file\E"
--
2.34.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox