Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: problems with patatt and stgit's cover letters #414

Closed
pcmoore opened this issue Feb 6, 2024 · 8 comments
Closed

BUG: problems with patatt and stgit's cover letters #414

pcmoore opened this issue Feb 6, 2024 · 8 comments

Comments

@pcmoore
Copy link

pcmoore commented Feb 6, 2024

I've been experiencing a problem when combining patatt with stacked-git's cover letters. Everything appears to work fine when sending single patches, or even multiple patches without a cover letter, but as soon as I attempt to send a cover letter, patatt fails with an error similar to below:

 % stg email send --to=guest@example.org  -a --compose
 ...
 error: `git send-email`: patatt: E:
   The following required headers not present: from

I've been in touch with the patatt developer to try and resolve the problem, but I thought I would also mention it here as there is some thinking that stacked-git may not be creating the cover letter properly ... or perhaps I'm screwing something up in my config, we shouldn't rule out the most likely source of error ;)

Any help you can provide would be appreciated, I'm also willing to provide whatever additional information you need to help diagnose the problem, just let me know.

The discussion with the patatt developer, @mricon, can be seen in the Linux kernel's tool archive link below:

@pcmoore
Copy link
Author

pcmoore commented Feb 6, 2024

For reference, here is the .git/hooks/sendemail-validate that I was using to execute patatt and which was causing the problem:

patatt sign --hook "${1}"

... here is the workaround I've put in place which appears to work:

head -n 1 "${1}" | grep -q "^From [0-9a-f]* " && exec patatt sign --hook "${1}"

@pcmoore
Copy link
Author

pcmoore commented Feb 6, 2024

I should add, this is with stacked-git v2.4.3 and git v2.43.0.

@mricon
Copy link

mricon commented Feb 6, 2024

Specifically, we narrowed it down to the following line in the cover letter:

From Paul Moore <paul@paul-moore.com>

(note the missing ":" after "From"). This is not a valid header, and I'm curious if it's generated by stgit or some other tool.

@jpgrayson
Copy link
Collaborator

jpgrayson commented Feb 7, 2024

Thanks for the issue report.

StGit's stg email send and stg email format commands are very thin wrappers around git send-email and git format-patch, respectively. The main thing they do is map StGit patch selections to commit hashes that git needs. Otherwise, StGit is just passing options through to git.

From taking a quick look at this, here is what I'm seeing in $EDITOR when I run stg email send --compose:

From Peter Grayson <pete@jpgrayson.net> # This line is ignored.
GIT: Lines beginning in "GIT:" will be removed.
GIT: Consider including an overall diffstat or table of contents
GIT: for the patch you are writing.
GIT: 
GIT: Clear the body content if you don't wish to send a summary.
From: Peter Grayson <pete@jpgrayson.net>
To: guest@example.org
Cc: 
Bcc: 
Reply-To: 
Subject: 
In-Reply-To: 

GIT: [PATCH] something

I note that the first line contains From Peter Grayson... without a ":". But further down on line 7 we see the proper email headers start, including From: Peter Grayson....

This is coming straight out of git send-email. I'm curious whether pattatt and the validation hook behave differently with the equivalent git send-email command?

N.B. in my case, the equivalent git command is:

git send-email --compose --to=guest@example.org HEAD~..HEAD

@mricon
Copy link

mricon commented Feb 7, 2024

I think the "From:" you see on line 7 is inserted by git-send-email using the user.name and user.email values from gitconfig -- because there are no valid From: headers in the message.

This only affects the cover letter, which isn't created by git-format-patch, so my guess would be that whatever creates the cover letter message is not writing the From: header correctly.

@jpgrayson
Copy link
Collaborator

Given that what we're seeing is behavior straight out of git send-email --compose, it's not clear that there are any changes that can or should be made to StGit to resolve this problem. I'm a little surprised that this issue with patatt validation has only come up in the context of StGit.

But, it also seems like a workflow using stg email format --cover-letter followed by stg email-send might be a viable path to sending patches with cover letters without triggering this problem. When I've sent patch series with cover letters in the past, that's the workflow I've used.

I'm not an expert in email-based workflows, so I'm not sure what the use cases are for git send-email --compose versus using git format-patch --cover-letter.

@pcmoore
Copy link
Author

pcmoore commented Feb 7, 2024

Given that what we're seeing is behavior straight out of git send-email --compose, it's not clear that there are any changes that can or should be made to StGit to resolve this problem. I'm a little surprised that this issue with patatt validation has only come up in the context of StGit.

I was surprised too.

But, it also seems like a workflow using stg email format --cover-letter followed by stg email-send might be a viable path to sending patches with cover letters without triggering this problem. When I've sent patch series with cover letters in the past, that's the workflow I've used.

I'm not an expert in email-based workflows, so I'm not sure what the use cases are for git send-email --compose versus using git format-patch --cover-letter.

I can only speak for my workflow, but when I'm sending multi-patch patchsets I always use stg email send --compose directly as my cover letters typically fall into one of two categories: trivial enough that I can draft it on the fly, or involved enough that I manage the cover letter text separately and copy-n-paste into the editor when sending the patch.

FWIW, I'm currently working around this issue with the revised sendemail-validate hook I posted earlier in this thread.

@jpgrayson
Copy link
Collaborator

I'm closing this issue. The problem seems to lie somewhere between the behavior of git send-email --compose, the sendemail-validate hook, and patatt.

I'll note that git's template sendemail-validate hook specifically tests whether the input file is a patch or a cover letter, it seems like that pattern might be appropriate for use with patatt and maybe explains why others aren't necessarily seeing this problem; i.e. if one were to start with git's template and only use patatt in the validate_patch branch, then this problem wouldn't happen.

Anyway, I don't see anything to do in StGit for this one.

mricon added a commit to mricon/patatt that referenced this issue Feb 14, 2024
When git-send-email is invoked with --compose, it will create a cover
letter template that is not a valid RFC2822 message by itself (e.g. it
has lines starting with "GIT: " that will be removed before the message
is sent). Refuse to sign such templated messages, because the contents
are going to be modified before the message is sent and the signature is
going to be invalid anyway.

Reported-by: Paul Moore <paul@paul-moore.com>
Link: stacked-git/stgit#414
Signed-off-by: Konstantin Ryabitsev <konstantin@linuxfoundation.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants