| Message ID | 20210421202942.1358011-1-foxboron@archlinux.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | b4: Ensure we read threadfile for message-id | expand |
Morten Linderud writes: > This fixes a bug where reading from `get_msgid_from_stdin` couldn't grab > the message-id when we collect a thread from stdin. > > This is mainly because there is no good way to override `sys.stdin` > (from what I can see) and it probably makes more sense to try fetch > message-ids from files instead. This allows us to replace the default > file "sys.stdin" with the thread file whenever we need. > > No patch: > > $ curl -s "https://lore.kernel.org/lkml/20210421130105.1226686-1-gregkh@linuxfoundation.org/raw" | b4 mbox > Looking up https://lore.kernel.org/r/20210421130105.1226686-1-gregkh%40linuxfoundation.org So the message ID is successfully read from stdin here... > Grabbing thread from lore.kernel.org/lkml > 272 messages in the thread > Unable to find a valid message-id in stdin. ... but then a subsequent call tries to read it from stdin again. > With patch: > > $ curl -s "https://lore.kernel.org/lkml/20210421130105.1226686-1-gregkh@linuxfoundation.org/raw" | .4 mbox typo: ".4" > Looking up https://lore.kernel.org/r/20210421130105.1226686-1-gregkh%40linuxfoundation.org > Grabbing thread from lore.kernel.org/lkml > 272 messages in the thread > Saved ./20210421130105.1226686-1-gregkh@linuxfoundation.org.mbx > > Signed-off-by: Morten Linderud <foxboron@archlinux.org> A similar error can still be triggered in the 'am --cherry-pick' code path: curl -s "https://lore.kernel.org/lkml/20210421130105.1226686-1-gregkh@linuxfoundation.org/raw" | b4 am -P _ Looking up https://lore.kernel.org/r/20210421130105.1226686-1-gregkh%40linuxfoundation.org Grabbing thread from lore.kernel.org/lkml Analyzing 276 messages in the thread --- Unable to find a valid message-id in stdin. I haven't tried, but it looks like you could update the get_msgid() call in mbox_to_am() to use mboxfile. Another approach would be to avoid collecting the msgid more than once (something like below). diff --git a/b4/mbox.py b/b4/mbox.py index d3bde25..3783a56 100644 --- a/b4/mbox.py +++ b/b4/mbox.py @@ -27,7 +27,7 @@ logger = b4.logger -def mbox_to_am(mboxfile, cmdargs): +def mbox_to_am(mboxfile, cmdargs, msgid): config = b4.get_main_config() outdir = cmdargs.outdir if outdir == '-': @@ -81,7 +81,6 @@ def mbox_to_am(mboxfile, cmdargs): if cmdargs.cherrypick: cherrypick = list() if cmdargs.cherrypick == '_': - msgid = b4.get_msgid(cmdargs) # Only grab the exact msgid provided at = 0 for lmsg in lser.patches[1:]: @@ -500,16 +499,14 @@ def main(cmdargs): savefile = mkstemp('b4-mbox')[1] + msgid = b4.get_msgid(cmdargs) if not cmdargs.localmbox: - msgid = b4.get_msgid(cmdargs) - threadfile = b4.get_pi_thread_by_msgid(msgid, savefile, useproject=cmdargs.useproject, nocache=cmdargs.nocache) if threadfile is None: os.unlink(savefile) return else: if os.path.exists(cmdargs.localmbox): - msgid = b4.get_msgid(cmdargs) if os.path.isdir(cmdargs.localmbox): in_mbx = mailbox.Maildir(cmdargs.localmbox) else: @@ -530,7 +527,7 @@ def main(cmdargs): get_extra_series(threadfile, direction=1) if cmdargs.subcmd == 'am': - mbox_to_am(threadfile, cmdargs) + mbox_to_am(threadfile, cmdargs, msgid) os.unlink(threadfile) return @@ -566,7 +563,6 @@ def main(cmdargs): if cmdargs.wantname: savefile = os.path.join(cmdargs.outdir, cmdargs.wantname) else: - msgid = b4.get_msgid(cmdargs) savefile = os.path.join(cmdargs.outdir, '%s.mbx' % msgid) mbx.close()
On Wed, Apr 21, 2021 at 07:33:55PM -0400, Kyle Meyer wrote: > I haven't tried, but it looks like you could update the get_msgid() call > in mbox_to_am() to use mboxfile. > > Another approach would be to avoid collecting the msgid more than once > (something like below). This is indeed a better approach, so I used it in the latest dev commit. Thanks to both of you. -K
diff --git a/b4/__init__.py b/b4/__init__.py index 32b5c02..e81d395 100644 --- a/b4/__init__.py +++ b/b4/__init__.py @@ -2179,18 +2179,18 @@ def get_requests_session(): return REQSESSION -def get_msgid_from_stdin(): +def get_msgid_from_file(file): if not sys.stdin.isatty(): - message = email.message_from_string(sys.stdin.read()) + message = email.message_from_string(file.read()) return message.get('Message-ID', None) logger.error('Error: pipe a message or pass msgid as parameter') sys.exit(1) -def get_msgid(cmdargs): +def get_msgid(cmdargs, file=sys.stdin): if not cmdargs.msgid: logger.debug('Getting Message-ID from stdin') - msgid = get_msgid_from_stdin() + msgid = get_msgid_from_file(file) if msgid is None: logger.error('Unable to find a valid message-id in stdin.') sys.exit(1) diff --git a/b4/mbox.py b/b4/mbox.py index d3bde25..791f545 100644 --- a/b4/mbox.py +++ b/b4/mbox.py @@ -566,7 +566,7 @@ def main(cmdargs): if cmdargs.wantname: savefile = os.path.join(cmdargs.outdir, cmdargs.wantname) else: - msgid = b4.get_msgid(cmdargs) + msgid = b4.get_msgid(cmdargs, file=open(threadfile)) savefile = os.path.join(cmdargs.outdir, '%s.mbx' % msgid) mbx.close()
This fixes a bug where reading from `get_msgid_from_stdin` couldn't grab the message-id when we collect a thread from stdin. This is mainly because there is no good way to override `sys.stdin` (from what I can see) and it probably makes more sense to try fetch message-ids from files instead. This allows us to replace the default file "sys.stdin" with the thread file whenever we need. No patch: $ curl -s "https://lore.kernel.org/lkml/20210421130105.1226686-1-gregkh@linuxfoundation.org/raw" | b4 mbox Looking up https://lore.kernel.org/r/20210421130105.1226686-1-gregkh%40linuxfoundation.org Grabbing thread from lore.kernel.org/lkml 272 messages in the thread Unable to find a valid message-id in stdin. With patch: $ curl -s "https://lore.kernel.org/lkml/20210421130105.1226686-1-gregkh@linuxfoundation.org/raw" | .4 mbox Looking up https://lore.kernel.org/r/20210421130105.1226686-1-gregkh%40linuxfoundation.org Grabbing thread from lore.kernel.org/lkml 272 messages in the thread Saved ./20210421130105.1226686-1-gregkh@linuxfoundation.org.mbx Signed-off-by: Morten Linderud <foxboron@archlinux.org> --- b4/__init__.py | 8 ++++---- b4/mbox.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-)