west patch: a unified way of handling patches

[mkschreder]

Is your feature request related to a problem? Please describe.

West is a fantastic tool for managing dependencies, however it seems that there isn't any good standardized way of using it to handle patches. A typical scenario is as follows:

• A project references Zephyr as well as other dependencies through a west.yml file.
• Developer works on a merge request in this repo and discovers a problem in a dependency
• In order to be able to proceed with the merge, developer creates a local patch for the external repository under some local "patches" directory.
• Developer can proceed with the merge and a separate merge request is then created in the dependency repository to merge the patch.
• Once patch is merged and version of the dependency is updated in the west.yml file, the patch file can be removed.

There are multiple ways to handle these patches including using "patch" command, "git apply" or "git am".

However, all these methods lack the ability to apply patches in the correct order based on hierarchy of west dependencies. Therefore I think that a more standardized west-based approach is needed.

One big problem is the case where a version of a dependency relies on having its own patches applied to another shared dependency that is also managed by west. Not having a standard process for this that is itself managed by west makes it very unclear which order the patches should be applied in.

Describe the solution you'd like
I would like to see patch management integrated into west as a new command "west patch", that will provide a standardized way of managing patches that works well with west.

• west patch: applies all patches in the correct order (what the correct order would be is up for discussion, but I think the hierarchy defined by the west.yml files can be used to derive the correct order)
• west patch --list: lists all patches
• west patch --reset: resets all patches
• west patch --diff: shows current uncommitted changes (similar to west diff)
• west patch --commit: commits changes and creates a new patch file under "patches/" in current project

Describe alternatives you've considered

• patch: works great when there are a few patches but becomes messy when multiple project use patches
• git apply: similar issues to using patch directly
• git am: better, but still does not take into account the bigger picture that is only visible to west.

[mkschreder]

Here is a starting point for a west plugin to do this:

from west.commands import WestCommand
from west import log
import git
from west.manifest import Manifest
from pathlib import Path


class WestPatch(WestCommand):
    def __init__(self):
        super().__init__(
            'patch',
            'manage patches across multiple repositories',
            dedent('''
            This command manages patches across multiple repositories.
            It can apply, list, reset, show diffs, and commit patches.
            ''')
        )

    def do_add_parser(self, parser_adder):
        parser = parser_adder.add_parser(
            self.name, help=self.help, description=self.description)
        parser.add_argument('--list', action='store_true',
                            help='list all patches')
        parser.add_argument('--reset', action='store_true',
                            help='reset all patches')
        parser.add_argument('--diff', action='store_true',
                            help='show current uncommitted changes')
        parser.add_argument('--commit', action='store_true',
                            help='commit changes and create a new patch file')
        return parser

    def do_run(self, args, unknown_args):
        if args.list:
            self.list_patches()
        elif args.reset:
            self.reset_patches()
        elif args.diff:
            self.show_diff()
        elif args.commit:
            self.commit_patches()
        else:
            self.apply_patches()

    def apply_patches(self):
        manifest = Manifest.from_file()
        for project in manifest.projects:
            patch_dir = Path(project.abspath) / 'patches'
            if patch_dir.exists():
                for patch_file in sorted(patch_dir.glob('*.patch')):
                    try:
                        project_repo = git.Repo(project.abspath)
                        project_repo.git.am(patch_file.as_posix())
                        log.inf(f'Applied {patch_file.name} to {project.name}')
                    except git.exc.GitCommandError as e:
                        log.err(f'Error applying {patch_file.name} to {project.name}: {e}')

    def list_patches(self):
        manifest = Manifest.from_file()
        for project in manifest.projects:
            patch_dir = Path(project.abspath) / 'patches'
            if patch_dir.exists():
                patches = sorted(patch_file.name for patch_file in patch_dir.glob('*.patch'))
                if patches:
                    log.inf(f'{project.name} has patches:')
                    for patch in patches:
                        log.inf(f'  - {patch}')
                else:
                    log.inf(f'{project.name} has no patches.')

    def reset_patches(self):
        manifest = Manifest.from_file()
        for project in manifest.projects:
            try:
                project_repo = git.Repo(project.abspath)
                project_repo.git.reset('--hard')
                project_repo.git.clean('-fdx')
                log.inf(f'Reset and cleaned {project.name}')
            except git.exc.GitCommandError as e:
                log.err(f'Error resetting {project.name}: {e}')

    def show_diff(self):
        manifest = Manifest.from_file()
        for project in manifest.projects:
            project_repo = git.Repo(project.abspath)
            diff = project_repo.git.diff()
            if diff:
                log.inf(f'Uncommitted changes in {project.name}:')
                log.inf(diff)
            else:
                log.inf(f'No uncommitted changes in {project.name}')

    def commit_patches(self):
        manifest = Manifest.from_file()
        for project in manifest.projects:
            project_repo = git.Repo(project.abspath)
            if project_repo.is_dirty(untracked_files=True):
                patch_dir = Path(project.abspath) / 'patches'
                patch_dir.mkdir(exist_ok=True)
                commit_message = input(f'Enter commit message for {project.name}: ')
                project_repo.git.add(A=True)
                project_repo.git.commit('-m', commit_message)
                patch_file = patch_dir / f'{project_repo.head.commit.hexsha[:7]}.patch'
                project_repo.git.format_patch('-1', '--stdout', output=patch_file.as_posix())
                log.inf(f'Committed changes and created patch {patch_file.name} for {project.name}')
            else:
                log.inf(f'No changes to commit for {project.name}')

If somebody can include this into the official plan for west improvements that can take into account integrating it perfectly into the west workflow then that would be great.

[benedekkupper]

@mkschreder thanks for the initiative and the great groundwork! Our project also needs this feature, as we really only need minor patches on zephyr & co (PRs are a pita), which aren't worth a full fork.

The main problem with the above solution is putting the patch files under the same git repository path, so on a second pass the patch files will be added into the changes of the next commit. I have polished the above solution further, now the patches land in the manifest repository folder, which is excluded from patching itself. Also, projects that aren't checked out are filtered.

from west.commands import WestCommand
from west import log
import git
from west.manifest import Manifest
from pathlib import Path
from textwrap import dedent

class WestPatch(WestCommand):
    def __init__(self):
        super().__init__(
            'patch',
            'manage patches across multiple repositories',
            dedent('''
            This command manages patches across multiple repositories.
            It can apply, list, show diffs, and commit patches.
            Reverting patches is done by "west update" command.
            ''')
        )
        self.manifest = Manifest.from_file()

    def do_add_parser(self, parser_adder):
        parser = parser_adder.add_parser(
            self.name, help=self.help, description=self.description)
        parser.add_argument('--list', action='store_true',
                            help='list all patches')
        parser.add_argument('--diff', action='store_true',
                            help='show current uncommitted changes')
        parser.add_argument('--commit', action='store_true',
                            help='commit changes and create a new patch file')
        return parser

    def do_run(self, args, unknown_args):
        if args.list:
            self.list_patches()
        elif args.diff:
            self.show_diff()
        elif args.commit:
            self.commit_patches()
        else:
            self.apply_patches()

    def get_git_projects(self):
        projects = []
        for project in self.manifest.projects:
            if project.name == 'manifest':
                continue # Skip the manifest project itself
            try:
                project_repo = git.Repo(project.abspath)
            except git.exc.NoSuchPathError as e:
                continue # Skip projects that aren't dependencies
            projects.append(project)
        return projects

    def get_patch_dir(self, project):
        return Path(self.manifest.repo_abspath) / 'patches' / project.name

    def apply_patches(self):
        for project in self.get_git_projects():
            patch_dir = self.get_patch_dir(project)
            if patch_dir.exists():
                for patch_file in sorted(patch_dir.glob('*.patch')):
                    try:
                        project_repo = git.Repo(project.abspath)
                        project_repo.git.am(patch_file.as_posix())
                        log.inf(f'Applied {patch_file.name} to {project.name}')
                    except git.exc.GitCommandError as e:
                        log.err(f'Error applying {patch_file.name} to {project.name}: {e}')

    def list_patches(self):
        for project in self.get_git_projects():
            patch_dir = self.get_patch_dir(project)
            if patch_dir.exists():
                patches = sorted(patch_file.name for patch_file in patch_dir.glob('*.patch'))
                if patches:
                    log.inf(f'{project.name} has patches:')
                    for patch in patches:
                        log.inf(f'  - {patch}')
                else:
                    log.inf(f'{project.name} has no patches.')

    def show_diff(self):
        for project in self.get_git_projects():
            project_repo = git.Repo(project.abspath)
            diff = project_repo.git.diff()
            if diff:
                log.inf(f'Uncommitted changes in {project.name}:')
                log.inf(diff)
            else:
                log.inf(f'No uncommitted changes in {project.name}')

    def commit_patches(self):
        for project in self.get_git_projects():
            project_repo = git.Repo(project.abspath)
            if project_repo.is_dirty(untracked_files=True):
                patch_dir = self.get_patch_dir(project)
                patch_dir.mkdir(exist_ok=True)
                commit_message = input(f'Enter commit message for {project.name}: ')
                project_repo.git.add(A=True)
                project_repo.git.commit('-m', commit_message)
                ret = project_repo.git.format_patch('-1', '-o', patch_dir.as_posix())
                log.inf(f'Committed changes and created patch {ret} for {project.name}')
            else:
                log.inf(f'No changes to commit for {project.name}')

With this in hand, the flow goes that after west update, a west patch applies the existing patches. Further changes can be committed and also exported to patch files with west patch --commit. Reverting the changes is a simple call of west update. The nice thing is that this makes patches portable across the team.