importer: fix handling of missing commits

[Patchew-devel] [PATCH] importer: fix handling of missing commits

Posted by Paolo Bonzini 6 years, 9 months ago

If the master branch is force-pushed and the old HEAD is not present in the
repository, patchew-cli is currently overwriting the old_head in an
attempt to make the subsequent "git log" command work.  However, this
caused a "wrong old head" exception in the project update command.

Instead, leave the old_head unchanged and use the new commits to build a
"git show" command line.  This works because the new commits are initialized
to a sane value if the master branch is force-pushed.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 patchew-cli | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/patchew-cli b/patchew-cli
index 73b8ccf..7fa7d49 100755
--- a/patchew-cli
+++ b/patchew-cli
@@ -308,15 +308,14 @@ class ProjectCommand(SubCommand):
                                                   "%s..%s" % (old_head, new_head)],
                                                   cwd=clone).decode().splitlines()
         except:
-            old_head = new_head
             new_commits = [new_head]
         logging.debug("old head: %s", old_head)
         logging.debug("new commits: \n%s" % \
                 "\n".join(["  " + x for x in new_commits]))
-        output = subprocess.check_output("""git log --format=%%b %s..%s |
+        output = subprocess.check_output("""git show --format=%%b %s |
                 awk 'BEGIN{IGNORECASE=1} /^message-id:/{print}'
                 """% \
-                (old_head, new_head), shell=True, cwd=clone).decode()
+                ' '.join(new_commits), shell=True, cwd=clone).decode()
         msgids = [x[len("message-id:"):].strip() for x in output.splitlines()]
         logging.debug("message ids: \n%s" % \
                 "\n".join(["  " + x for x in msgids]))
-- 
2.20.1

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel

Re: [Patchew-devel] [PATCH] importer: fix handling of missing commits

Posted by Fam Zheng 6 years, 9 months ago

On Fri, 03/15 17:51, Paolo Bonzini wrote:
> If the master branch is force-pushed and the old HEAD is not present in the
> repository, patchew-cli is currently overwriting the old_head in an
> attempt to make the subsequent "git log" command work.  However, this
> caused a "wrong old head" exception in the project update command.
> 
> Instead, leave the old_head unchanged and use the new commits to build a
> "git show" command line.  This works because the new commits are initialized
> to a sane value if the master branch is force-pushed.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  patchew-cli | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/patchew-cli b/patchew-cli
> index 73b8ccf..7fa7d49 100755
> --- a/patchew-cli
> +++ b/patchew-cli
> @@ -308,15 +308,14 @@ class ProjectCommand(SubCommand):
>                                                    "%s..%s" % (old_head, new_head)],
>                                                    cwd=clone).decode().splitlines()
>          except:
> -            old_head = new_head
>              new_commits = [new_head]
>          logging.debug("old head: %s", old_head)
>          logging.debug("new commits: \n%s" % \
>                  "\n".join(["  " + x for x in new_commits]))
> -        output = subprocess.check_output("""git log --format=%%b %s..%s |
> +        output = subprocess.check_output("""git show --format=%%b %s |
>                  awk 'BEGIN{IGNORECASE=1} /^message-id:/{print}'
>                  """% \
> -                (old_head, new_head), shell=True, cwd=clone).decode()
> +                ' '.join(new_commits), shell=True, cwd=clone).decode()

How does this make sure all message-ids in each commits between old_head and
new_head are collected? That list is used by the server to mark multiple series
merged.

Fam

>          msgids = [x[len("message-id:"):].strip() for x in output.splitlines()]
>          logging.debug("message ids: \n%s" % \
>                  "\n".join(["  " + x for x in msgids]))
> -- 
> 2.20.1
> 
> _______________________________________________
> Patchew-devel mailing list
> Patchew-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/patchew-devel

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel

Re: [Patchew-devel] [PATCH] importer: fix handling of missing commits

Posted by Paolo Bonzini 6 years, 9 months ago

On 17/03/19 03:56, Fam Zheng wrote:
>> -        output = subprocess.check_output("""git log --format=%%b %s..%s |
>> +        output = subprocess.check_output("""git show --format=%%b %s |
>>                  awk 'BEGIN{IGNORECASE=1} /^message-id:/{print}'
>>                  """% \
>> -                (old_head, new_head), shell=True, cwd=clone).decode()
>> +                ' '.join(new_commits), shell=True, cwd=clone).decode()
> How does this make sure all message-ids in each commits between old_head and
> new_head are collected? That list is used by the server to mark multiple series
> merged.

It doesn't.  On the other hand this only happens if someone force-pushes
to master, and the importer is redeployed in such a way that the
old_head is not part of the importer's clone.  Given the circumstances,
IMHO this is neither a big issue, nor a fully fixable issue.

On the other hand it can happen, and it is the cause of the broken
next.patchew.org git imports (as I found out after dropping all the
template crap from the logs...) so it's important to do _something_
about it.

Paolo

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel

Re: [Patchew-devel] [PATCH] importer: fix handling of missing commits

Posted by Fam Zheng 6 years, 9 months ago


> On Mar 18, 2019, at 16:46, Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> On 17/03/19 03:56, Fam Zheng wrote:
>>> -        output = subprocess.check_output("""git log --format=%%b %s..%s |
>>> +        output = subprocess.check_output("""git show --format=%%b %s |
>>>                 awk 'BEGIN{IGNORECASE=1} /^message-id:/{print}'
>>>                 """% \
>>> -                (old_head, new_head), shell=True, cwd=clone).decode()
>>> +                ' '.join(new_commits), shell=True, cwd=clone).decode()
>> How does this make sure all message-ids in each commits between old_head and
>> new_head are collected? That list is used by the server to mark multiple series
>> merged.
> 
> It doesn't.  On the other hand this only happens if someone force-pushes
> to master, and the importer is redeployed in such a way that the
> old_head is not part of the importer's clone.  Given the circumstances,
> IMHO this is neither a big issue, nor a fully fixable issue.
> 
> On the other hand it can happen, and it is the cause of the broken
> next.patchew.org git imports (as I found out after dropping all the
> template crap from the logs...) so it's important to do _something_
> about it.

OK, I see the logic now. Thanks.

Reviewed-by: Fam Zheng <famz@redhat.com>



_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel