tools.linux.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Kyle Meyer <kyle@kyleam.com>
To: Morten Linderud <foxboron@archlinux.org>
Cc: tools@linux.kernel.org
Subject: Re: b4: Ensure we read threadfile for message-id
Date: Wed, 21 Apr 2021 19:33:55 -0400	[thread overview]
Message-ID: <87im4fi55o.fsf@kyleam.com> (raw)
In-Reply-To: <20210421202942.1358011-1-foxboron@archlinux.org>

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()

  reply	other threads:[~2021-04-21 23:40 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-21 20:29 b4: Ensure we read threadfile for message-id Morten Linderud
2021-04-21 23:33 ` Kyle Meyer [this message]
2021-05-14 20:55   ` Konstantin Ryabitsev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87im4fi55o.fsf@kyleam.com \
    --to=kyle@kyleam.com \
    --cc=foxboron@archlinux.org \
    --cc=tools@linux.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).