Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions pypasta/JailhouseMailCharacteristics.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
"""
PaStA - Patch Stack Analysis

Copyright (c) OTH Regensburg, 2021
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OTH is not the copyright holder in this case. So this is either your university, or you.


Author:
Sebastian Duda <git@sebdu.de>

This work is licensed under the terms of the GNU GPL, version 2. See
the COPYING file in the top-level directory.
"""

from .MailCharacteristics import MailCharacteristics, PatchType


class JailhouseMailCharacteristics(MailCharacteristics):
ROOT_DIRS = ['ci',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hier müsstest du mir noch das Skript zukommen lassen, mit dem du alle Versionen überprüft hast.
Andernfalls scripte ich das selbst.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, just sent it to you. The directories seem complete!

'configs',
'Documentation',
'driver',
'hypervisor',
'include',
'inmates',
'pyjailhouse',
'scripts',
'tools',
]
ROOT_FILES = ['CONTRIBUTING.md',
'COPYING',
'FAQ.md',
'.git',
'.gitignore',
'Kbuild',
'LICENSING.md',
'Makefile',
'README.md',
'setup.py',
'TODO.md',
'.travis.yml',
'VERSION',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We miss:

driver.c
jailhouse.h
README (there was an ancient version without .md)
TODO (same here)

Rest looks good!

]

# Additional lists that are not known by pasta
LISTS = set()

HAS_MAINTAINERS = False

def __init__(self, repo, maintainers_version, clustering, message_id):
super().__init__(repo, clustering, message_id)
self.__init(repo, maintainers_version)
self._cleanup()

def __init(self, repo, maintainers_version):
if self.is_from_bot:
self.type = PatchType.BOT

if not self.is_patch:
return

if self.type == PatchType.OTHER:
self.type = PatchType.PATCH

self._integrated_correct(repo, maintainers_version)
42 changes: 33 additions & 9 deletions pypasta/MailCharacteristics.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ class MailCharacteristics:
BOTS = {'tip-bot2@linutronix.de', 'tipbot@zytor.com',
'noreply@ciplatform.org', 'patchwork@emeril.freedesktop.org'}
POTENTIAL_BOTS = {'broonie@kernel.org', 'lkp@intel.com'}
PROCESSES = ['linux-next', 'git pull', 'rfc']
PROCESSES = ['linux-next', 'git pull', 'rfc', '[PULL]']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, sounds valid. But what about having everything here in lower case, and comparing it against lower case? With this, we would also catch [pull].


@staticmethod
def dump_release_info(config):
Expand Down Expand Up @@ -171,12 +171,31 @@ def _is_from_bot(self):
if self.is_next and 'sfr@canb.auug.org.au' in email:
return True

# Github Bot
if 'noreply@github.com' in email:
return True

# Buildroot's daily results bot
if '[autobuild.buildroot.net] Daily results' in subject:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please compare against subject.lower()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not necessary since subject = email_get_header_normalised(self.message, 'subject') already lowers the subject.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, you're right - but then 'Daily results' will never match.

return True

# Buildroot's daily results bot
if 'oe-core cve metrics' in subject.lower():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And then you can combine those two cases.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides that, looks good!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And the .lower() is redundant here.

return True

return False

def _integrated_correct(self, repo, maintainers_version):
if self.first_upstream in repo:
upstream = repo[self.first_upstream]
self.committer = upstream.committer.name.lower()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wollen wir hier vielleicht die Mailadresse ausleiten? Die ist aussagekräftiger, oder?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I remember that - for some reason - we talked about this change, but I don't understand at the moment why we did this. Could you please factor this change out to an own commit with an explanation why we do this? This change is preparatory work for supporting other projects, and not related to the introduction of Jailhouse.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need this to get the committer of patches in projects without a maintainer.
Without this change, we would return from this function before we get this information.
The information about the committer however is relevant and valid for all projects.


if maintainers_version is None:
return

# stuff for maintainers analysis
self.maintainers = dict()

maintainers = maintainers_version[self.version]
sections = maintainers.get_sections_by_files(self.patch.diff.affected)
for section in sections:
Expand All @@ -197,8 +216,6 @@ def check_maintainer(section, committer):
# integrated by a maintainer that is responsible for a section that is
# affected by the patch. IOW: The field indicates if the patch was
# picked by the "correct" maintainer
upstream = repo[self.first_upstream]
self.committer = upstream.committer.name.lower()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And what about the above comments? They now point into the void?

self.integrated_correct = False
self.integrated_xcorrect = False
sections = maintainers.get_sections_by_files(upstream.diff.affected)
Expand Down Expand Up @@ -236,6 +253,9 @@ def get_cluster(section):
break

def list_matches_patch(self, list):
if not self.maintainers:
return None

for lists, _, _ in self.maintainers.values():
if list in lists:
return True
Expand All @@ -262,7 +282,7 @@ def __init__(self, repo, clustering, message_id):
self.lists = repo.mbox.get_lists(message_id)

# stuff for maintainers analysis
self.maintainers = dict()
self.maintainers = None

# Patch characteristics
self.is_patch = message_id in repo and message_id not in repo.mbox.invalid
Expand Down Expand Up @@ -327,11 +347,13 @@ def load_maintainers_characteristics(config, characteristics_class, clustering,
ids):
repo = config.repo

# We can safely limit to patches only, as only patches will be used for the
# maintainers analysis.
patches = ids - repo.mbox.invalid
tags = {repo.patch_get_version(repo[x]) for x in patches}
maintainers_version = load_maintainers(config, tags)
maintainers_version = None
if characteristics_class.HAS_MAINTAINERS:
# We can safely limit to patches only, as only patches will be used for the
# maintainers analysis.
patches = ids - repo.mbox.invalid
tags = {repo.patch_get_version(repo[x]) for x in patches}
maintainers_version = load_maintainers(config, tags)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about all other projects that do not have HAS_MAINTAINERS yet? They are now broken with this change. Please factor this change out to a preparatory patch, then we get a better overview how we can handle this.


def _load_characteristics(ret):
if ret is None:
Expand Down Expand Up @@ -373,11 +395,13 @@ def load_characteristics(config, clustering, message_ids = None):
config.mbox_timewindow, and loads multiple instances of maintainers for the
patches of the clustering.
"""
from .JailhouseMailCharacteristics import JailhouseMailCharacteristics
from .LinuxMailCharacteristics import LinuxMailCharacteristics
from .QemuMailCharacteristics import QemuMailCharacteristics
from .UBootMailCharacteristics import UBootMailCharacteristics
from .XenMailCharacteristics import XenMailCharacteristics
_load_characteristics = {
'jailhouse': (load_maintainers_characteristics, JailhouseMailCharacteristics),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And if we once have factored out those things, we might even see a better way to handle this dictionary here.

'linux': (load_maintainers_characteristics, LinuxMailCharacteristics),
'qemu': (load_maintainers_characteristics, QemuMailCharacteristics),
'u-boot': (load_maintainers_characteristics, UBootMailCharacteristics),
Expand Down
1 change: 1 addition & 0 deletions pypasta/Repository/Repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
_tmp_repo = None

mainline_regex = {
'jailhouse': re.compile(r'^v.*$'),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

'linux': re.compile(r'^v(\d+\.\d+|2\.6\.\d+)(-rc\d+)?$'),
'qemu': re.compile(r'^v.*$'),
'u-boot': re.compile(r'^v201.*$'),
Expand Down