Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add b4 Tool #3506

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+103 −9
Open

Add b4 Tool #3506

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
54a1749
Git Tool: Add support for .mbx patches
adityagesh Nov 12, 2024
6557afe
Add B4 Tool
adityagesh Nov 12, 2024
a3f9480
Add b4 Kernel Source Modifier
adityagesh Nov 15, 2024
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
2 changes: 2 additions & 0 deletions lisa/tools/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
)

from .aria import Aria
from .b4 import B4
from .blkid import Blkid
from .bzip2 import Bzip2
from .cargo import Cargo
Expand Down Expand Up @@ -129,6 +130,7 @@
__all__ = [
"AptAddRepository",
"Aria",
"B4",
"Blkid",
"Bzip2",
"Cargo",
Expand Down
54 changes: 54 additions & 0 deletions lisa/tools/b4.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import pathlib
import re
from typing import List, Type

from lisa.executable import Tool
from lisa.operating_system import Debian
from lisa.tools.git import Git
from lisa.tools.python import Pip
from lisa.util import LisaException, find_group_in_lines


class B4(Tool):
# Output log is of the form
# git am /mnt/code/linux/v2_20241029_xxx_offers.mbx
_output_file_pattern = re.compile(
r"^.*git.*/(?P<filename>[\w-]+\.mbx).*$", re.MULTILINE
)

@property
def command(self) -> str:
return "b4"

@property
def dependencies(self) -> List[Type[Tool]]:
return [Git]

@property
def can_install(self) -> bool:
return True

def _install(self) -> bool:
if isinstance(self.node.os, Debian):
self.node.os.install_packages("b4")
installed = self._check_exists()
if not installed:
pip = self.node.tools[Pip]
pip.install_packages("b4", install_to_user=True)
return self._check_exists()

def am(
self, message_id: str, output_dir: pathlib.PurePath, sudo: bool = False
) -> pathlib.PurePath:
result = self.run(
f"am -o '{output_dir}' '{message_id}'",
force_run=True,
expected_exit_code=0,
sudo=sudo,
)
filename = find_group_in_lines(
lines=result.stdout, pattern=self._output_file_pattern, single_line=False
).get("filename")
if not filename:
raise LisaException("Failed to get filename from b4 am output")
return pathlib.PurePath(output_dir, filename)
27 changes: 19 additions & 8 deletions lisa/tools/git.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,14 +191,25 @@ def apply(
cwd: pathlib.PurePath,
patches: pathlib.PurePath,
) -> None:
result = self.run(
f"apply {patches}",
shell=True,
cwd=cwd,
force_run=True,
no_info_log=True,
no_error_log=True,
)
file_extension = patches.suffix
if file_extension == ".mbx":
Copy link
Member

Choose a reason for hiding this comment

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

I just understood the scenario. Because the .mbx is always generated by b4, and the call sequence is always b4.am and git.apply. So the b4 can be called in git, or call git in b4, so it's easier to use.

Copy link
Collaborator Author

@adityagesh adityagesh Nov 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

How about something like this?

class Git(Tool):
  def apply_patch(self,....):
      # handles both apply and am
      
class B4(Tool):
  def am(self,....):
    self.run("am {patches}", ....)
    
  def apply(self,....):
      git = node.tool[Git]
      self.am()
      git.apply()

Copy link
Member

Choose a reason for hiding this comment

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

  1. The apply_patch and apply are confusing in the Git tool.
  2. If call B4 in git, so the API can handle different types of patch files automatically. But if call git in B4, it still needs to know the type of patch files, and then call either git.apply or b4.apply.

Copy link
Collaborator Author

@adityagesh adityagesh Nov 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The tool b4 has dependency on Git while pulling patches (taking Message_id as the parameter), so I don't think we can add b4 to Git.

Currently in Git.apply, it takes the path as parameter, which is basically the operation after we download the patch using b4.
B4 after downloading the patch should always call git apply to apply the patch.

The b4 apply that I showed above is : download patch using b4 -> apply downloaded patch using git

Copy link
Member

Choose a reason for hiding this comment

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

  1. I saw the code mark Git as a dependency. It's not necessary, because it's always used after Git tool is called.
  2. The dependencies between b4 and git, are depended by how we manage them. The goal is to provide easy to use interface. If we provide B4 supports in git, the user doesn't need to take care the ext name, just call git to fit different file types. But if the B4 call git, it needs to know ext name firstly.

Copy link
Collaborator Author

@adityagesh adityagesh Nov 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Are you recommending adding something like below in Git?

download_and_apply_patch(self, Message_Id):
 path = b4.am(Message_id)
 git.apply_patch(path)

I saw the code mark Git as a dependency. It's not necessary, because it's always used after Git tool is called.

If we have the tool B4, it is possible someone uses it directly before invoking git. b4 will fail is git is not installed and not set as a dependency

Copy link
Member

Choose a reason for hiding this comment

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

It's like below. There is no conflict to block direct use b4.

    def apply(
        self,
        cwd: pathlib.PurePath,
        patches: pathlib.PurePath,
    ) -> None:
        file_extension = patches.suffix
        if file_extension == ".<b4's ext>"
            from .b4 import B4
            b4 = self.node.tools[B4]
            b4.am(...
            self.run(...
        else:
            self.run(...

result = self.run(
f"am {patches}",
shell=True,
cwd=cwd,
force_run=True,
no_info_log=True,
no_error_log=True,
)
else:
result = self.run(
f"apply {patches}",
shell=True,
cwd=cwd,
force_run=True,
no_info_log=True,
no_error_log=True,
)
result.assert_exit_code(message=f"failed on applying patches. {result.stdout}")

def list_tags(self, cwd: pathlib.PurePath) -> List[str]:
Expand Down
29 changes: 28 additions & 1 deletion lisa/transformers/kernel_source_installer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from lisa.base_tools import Mv
from lisa.node import Node
from lisa.operating_system import CBLMariner, Redhat, Ubuntu
from lisa.tools import Cp, Echo, Git, Make, Sed, Uname
from lisa.tools import B4, Cp, Echo, Git, Make, Sed, Uname
from lisa.tools.gcc import Gcc
from lisa.tools.lscpu import Lscpu
from lisa.util import LisaException, field_metadata, subclasses
Expand Down Expand Up @@ -75,6 +75,12 @@ class PatchModifierSchema(BaseModifierSchema):
file_pattern: str = "*.patch"


@dataclass_json()
@dataclass
class B4PatchModifierSchema(BaseModifierSchema):
message_id: str = field(default="", metadata=field_metadata(required=True))


@dataclass_json()
@dataclass
class SourceInstallerSchema(BaseInstallerSchema):
Expand Down Expand Up @@ -489,3 +495,24 @@ def _get_code_path(path: str, node: Node, default_name: str) -> PurePath:
code_path = node.working_path / default_name

return code_path


class B4PatchModifier(BaseModifier):
@classmethod
def type_name(cls) -> str:
return "b4_patch"

@classmethod
def type_schema(cls) -> Type[schema.TypedSchema]:
return B4PatchModifierSchema

def modify(self) -> None:
runbook: B4PatchModifierSchema = self.runbook

git = self._node.tools[Git]
b4 = self._node.tools[B4]

message_id = runbook.message_id
patch_file = b4.am(message_id=message_id, output_dir=self._code_path)

git.apply(cwd=self._code_path, patches=patch_file)
Loading