~postmarketos/pmbootstrap-devel

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
5 4

[PATCH pmbootstrap] flasher: add support for '--no-reboot' and '--resume' for heimdall-bootimg and fastboot

Details
Message ID
<170240028195.15140.12385625685092383241-0@git.sr.ht>
DKIM signature
missing
Download raw message
Patch: +47 -5
From: Andras Sebok <sebokandris2009@gmail.com>

Example usecase:
pmbootstrap flasher --no-reboot flash_kernel
pmbootstrap flasher --no-reboot --resume flash_rootfs
pmbootstrap flasher --resume flash_vbmeta

Closes: https://gitlab.com/postmarketOS/pmbootstrap/-/issues/2258
---
 pmb/config/__init__.py   | 22 ++++++++++++++++++----
 pmb/flasher/run.py       | 15 +++++++++++++++
 pmb/flasher/variables.py | 12 +++++++++++-
 pmb/parse/arguments.py   |  3 +++
 4 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/pmb/config/__init__.py b/pmb/config/__init__.py
index 05fba846..383092ca 100644
--- a/pmb/config/__init__.py
+++ b/pmb/config/__init__.py
@@ -924,6 +924,7 @@ $IMAGE_SPLIT_BOOT: Path to the (split) boot image
$IMAGE_SPLIT_ROOT: Path to the (split) rootfs image
$PARTITION_KERNEL: Partition to flash the kernel/boot.img to
$PARTITION_ROOTFS: Partition to flash the rootfs to
$REBOOT: Set to --no-reboot if the device should not be rebooted

Fastboot specific: $KERNEL_CMDLINE
Heimdall specific: $PARTITION_INITFS
@@ -932,6 +933,8 @@ uuu specific: $UUU_SCRIPT
flashers = {
    "fastboot": {
        "depends": [],  # pmaports.cfg: supported_fastboot_depends
        "no_reboot_supported": True,
        "reboot_command": ["fastboot", "reboot"],
        "actions": {
            "list_devices": [["fastboot", "devices", "-l"]],
            "flash_rootfs": [["fastboot", "flash", "$PARTITION_ROOTFS",
@@ -963,6 +966,8 @@ flashers = {
    "fastboot-bootpart": {
        "split": True,
        "depends": ["android-tools"],
        "no_reboot_supported": True,
        "reboot_command": ["fastboot", "reboot"],
        "actions": {
            "list_devices": [["fastboot", "devices", "-l"]],
            "flash_rootfs": [["fastboot", "flash", "$PARTITION_ROOTFS",
@@ -979,6 +984,7 @@ flashers = {
    # "isorec" (isolated recovery), a term coined by Lanchon.
    "heimdall-isorec": {
        "depends": ["heimdall"],
        "no_reboot_supported": False,
        "actions": {
            "list_devices": [["heimdall", "detect"]],
            "flash_rootfs": [
@@ -994,28 +1000,33 @@ flashers = {
    # fastboot compatible devices. Example: s7562, n7100
    "heimdall-bootimg": {
        "depends": [],  # pmaports.cfg: supported_heimdall_depends
        "no_reboot_supported": True,
        "actions": {
            "list_devices": [["heimdall", "detect"]],
            "flash_rootfs": [
                ["heimdall_wait_for_device.sh"],
                ["heimdall", "flash", "--$PARTITION_ROOTFS", "$IMAGE"]],
                ["heimdall", "flash", "--$PARTITION_ROOTFS", "$IMAGE",
                 "$REBOOT", "$RESUME"]],
            "flash_kernel": [
                ["heimdall_wait_for_device.sh"],
                ["heimdall", "flash", "--$PARTITION_KERNEL",
                 "$BOOT/boot.img$FLAVOR"]],
                 "$BOOT/boot.img$FLAVOR", "$REBOOT", "$RESUME"]],
            "flash_vbmeta": [
                ["avbtool", "make_vbmeta_image", "--flags", "2",
                    "--padding_size", "$FLASH_PAGESIZE",
                    "--output", "/vbmeta.img"],
                ["heimdall", "flash", "--$PARTITION_VBMETA", "/vbmeta.img"],
                ["heimdall", "flash", "--$PARTITION_VBMETA", "/vbmeta.img",
                 "$REBOOT", "$RESUME"],
                ["rm", "-f", "/vbmeta.img"]],
            "flash_lk2nd": [
                ["heimdall_wait_for_device.sh"],
                ["heimdall", "flash", "--$PARTITION_KERNEL", "$BOOT/lk2nd.img"]]
                ["heimdall", "flash", "--$PARTITION_KERNEL", "$BOOT/lk2nd.img",
                 "$REBOOT", "$RESUME"]]
        },
    },
    "adb": {
        "depends": ["android-tools"],
        "no_reboot_supported": False,
        "actions": {
            "list_devices": [["adb", "-P", "5038", "devices"]],
            "sideload": [["echo", "< wait for any device >"],
@@ -1026,6 +1037,7 @@ flashers = {
    },
    "uuu": {
        "depends": ["nxp-mfgtools-uuu"],
        "no_reboot_supported": False,
        "actions": {
            "flash_rootfs": [
                # There's a bug(?) in uuu where it clobbers the path in the cmd
@@ -1038,6 +1050,7 @@ flashers = {
    "rkdeveloptool": {
        "split": True,
        "depends": ["rkdeveloptool"],
        "no_reboot_supported": False,
        "actions": {
            "list_devices": [["rkdeveloptool", "list"]],
            "flash_rootfs": [
@@ -1052,6 +1065,7 @@ flashers = {
    },
    "mtkclient": {
        "depends": ["mtkclient"],
        "no_reboot_supported": False,
        "actions": {
            "flash_rootfs": [["mtk", "w", "$PARTITION_ROOTFS",
                              "$IMAGE"]],
diff --git a/pmb/flasher/run.py b/pmb/flasher/run.py
index 7daa7376..f66d9e2e 100644
--- a/pmb/flasher/run.py
+++ b/pmb/flasher/run.py
@@ -51,6 +51,19 @@ def run(args, action, flavor=None):
                           " <https://wiki.postmarketos.org/wiki/"
                           "Deviceinfo_reference>")

    if (not args.reboot and "flash" not in action) or \
        (not args.reboot and not cfg["no_reboot_supported"]):
        raise RuntimeError("This action does not support "
                           "the '--no-reboot' argument.")
    
    if (not args.resume and "flash" not in action) or \
        (not args.resume and not cfg["no_reboot_supported"]):
        raise RuntimeError("This action does not support "
                           "the '--resume' argument.")
    
    if args.reboot and "reboot_command" in cfg and "flash" in action:
        cfg["actions"][action].append(cfg["reboot_command"])

    # Run the commands of each action
    for command in cfg["actions"][action]:
        # Variable replacement
@@ -66,5 +79,7 @@ def run(args, action, flavor=None):
                    check_partition_blacklist(args, key, value)
                    command[i] = command[i].replace(key, value)

        # Remove empty strings
        command = [x for x in command if x != '']
        # Run the action
        pmb.chroot.root(args, command, output="interactive")
diff --git a/pmb/flasher/variables.py b/pmb/flasher/variables.py
index 6fd5469a..3cba6685 100644
--- a/pmb/flasher/variables.py
+++ b/pmb/flasher/variables.py
@@ -62,6 +62,14 @@ def variables(args, flavor, method):
    if args.deviceinfo["append_dtb"] == "true":
        _dtb = "-dtb"

    _reboot = ""
    if not args.reboot:
        _reboot = "--no-reboot"
    
    _resume = ""
    if args.resume:
        _resume = "--resume"

    vars = {
        "$BOOT": "/mnt/rootfs_" + args.device + "/boot",
        "$DTB": _dtb,
@@ -80,7 +88,9 @@ def variables(args, flavor, method):
                         "/var/lib/postmarketos-android-recovery-installer"
                         "/pmos-" + args.device + ".zip",
        "$UUU_SCRIPT": "/mnt/rootfs_" + args.deviceinfo["codename"] +
                       "/usr/share/uuu/flash_script.lst"
                       "/usr/share/uuu/flash_script.lst",
        "$REBOOT": _reboot,
        "$RESUME": _resume
    }

    # Backwards compatibility with old mkinitfs (pma#660)
diff --git a/pmb/parse/arguments.py b/pmb/parse/arguments.py
index ffd8780c..92f2fbf1 100644
--- a/pmb/parse/arguments.py
+++ b/pmb/parse/arguments.py
@@ -230,6 +230,9 @@ def arguments_flasher(subparser):
                               " target device")
    ret.add_argument("--method", help="override flash method",
                     dest="flash_method", default=None)
    ret.add_argument("--no-reboot", help="do not reboot after flashing", dest="reboot", action="store_false")
    ret.add_argument("--resume", help="perform another action after using --no-reboot", dest="resume", action="store_true")

    sub = ret.add_subparsers(dest="action_flasher")
    sub.required = True

-- 
2.38.5

[pmbootstrap/patches/.build.yml] build success

builds.sr.ht <builds@sr.ht>
Details
Message ID
<CXMIUNIQJZP4.3O4ATMVA2R2BS@cirno2>
In-Reply-To
<170240028195.15140.12385625685092383241-0@git.sr.ht> (view parent)
DKIM signature
missing
Download raw message
pmbootstrap/patches/.build.yml: SUCCESS in 14m52s

[flasher: add support for '--no-reboot' and '--resume' for heimdall-bootimg and fastboot][0] from [~andrisas][1]

[0]: https://lists.sr.ht/~postmarketos/pmbootstrap-devel/patches/47671
[1]: sebokandris2009@gmail.com

✓ #1113510 SUCCESS pmbootstrap/patches/.build.yml https://builds.sr.ht/~postmarketos/job/1113510

Re: [PATCH pmbootstrap] flasher: add support for '--no-reboot' and '--resume' for heimdall-bootimg and fastboot

Details
Message ID
<5734701.DvuYhMxLoT@z3ntu.xyz>
In-Reply-To
<170240028195.15140.12385625685092383241-0@git.sr.ht> (view parent)
DKIM signature
missing
Download raw message
On Dienstag, 12. Dezember 2023 17:54:05 CET ~andrisas wrote:
> From: Andras Sebok <sebokandris2009@gmail.com>
> 
> Example usecase:
> pmbootstrap flasher --no-reboot flash_kernel
> pmbootstrap flasher --no-reboot --resume flash_rootfs
> pmbootstrap flasher --resume flash_vbmeta
> 
> Closes: https://gitlab.com/postmarketOS/pmbootstrap/-/issues/2258

Couldn't we always use this no-reboot mode in heimdall? The fastboot flash 
also keeps you in fastboot until you run fastboot reboot or somehow else 
reboot your device. That would make it more consistent there.
Or is there some "user knowledge" required regarding using whatever --resume 
does?

Regards
Luca


> ---
>  pmb/config/__init__.py   | 22 ++++++++++++++++++----
>  pmb/flasher/run.py       | 15 +++++++++++++++
>  pmb/flasher/variables.py | 12 +++++++++++-
>  pmb/parse/arguments.py   |  3 +++
>  4 files changed, 47 insertions(+), 5 deletions(-)
> 
> diff --git a/pmb/config/__init__.py b/pmb/config/__init__.py
> index 05fba846..383092ca 100644
> --- a/pmb/config/__init__.py
> +++ b/pmb/config/__init__.py
> @@ -924,6 +924,7 @@ $IMAGE_SPLIT_BOOT: Path to the (split) boot image
>  $IMAGE_SPLIT_ROOT: Path to the (split) rootfs image
>  $PARTITION_KERNEL: Partition to flash the kernel/boot.img to
>  $PARTITION_ROOTFS: Partition to flash the rootfs to
> +$REBOOT: Set to --no-reboot if the device should not be rebooted
> 
>  Fastboot specific: $KERNEL_CMDLINE
>  Heimdall specific: $PARTITION_INITFS
> @@ -932,6 +933,8 @@ uuu specific: $UUU_SCRIPT
>  flashers = {
>      "fastboot": {
>          "depends": [],  # pmaports.cfg: supported_fastboot_depends
> +        "no_reboot_supported": True,
> +        "reboot_command": ["fastboot", "reboot"],
>          "actions": {
>              "list_devices": [["fastboot", "devices", "-l"]],
>              "flash_rootfs": [["fastboot", "flash", "$PARTITION_ROOTFS",
> @@ -963,6 +966,8 @@ flashers = {
>      "fastboot-bootpart": {
>          "split": True,
>          "depends": ["android-tools"],
> +        "no_reboot_supported": True,
> +        "reboot_command": ["fastboot", "reboot"],
>          "actions": {
>              "list_devices": [["fastboot", "devices", "-l"]],
>              "flash_rootfs": [["fastboot", "flash", "$PARTITION_ROOTFS",
> @@ -979,6 +984,7 @@ flashers = {
>      # "isorec" (isolated recovery), a term coined by Lanchon.
>      "heimdall-isorec": {
>          "depends": ["heimdall"],
> +        "no_reboot_supported": False,
>          "actions": {
>              "list_devices": [["heimdall", "detect"]],
>              "flash_rootfs": [
> @@ -994,28 +1000,33 @@ flashers = {
>      # fastboot compatible devices. Example: s7562, n7100
>      "heimdall-bootimg": {
>          "depends": [],  # pmaports.cfg: supported_heimdall_depends
> +        "no_reboot_supported": True,
>          "actions": {
>              "list_devices": [["heimdall", "detect"]],
>              "flash_rootfs": [
>                  ["heimdall_wait_for_device.sh"],
> -                ["heimdall", "flash", "--$PARTITION_ROOTFS", "$IMAGE"]],
> +                ["heimdall", "flash", "--$PARTITION_ROOTFS", "$IMAGE",
> +                 "$REBOOT", "$RESUME"]],
>              "flash_kernel": [
>                  ["heimdall_wait_for_device.sh"],
>                  ["heimdall", "flash", "--$PARTITION_KERNEL",
> -                 "$BOOT/boot.img$FLAVOR"]],
> +                 "$BOOT/boot.img$FLAVOR", "$REBOOT", "$RESUME"]],
>              "flash_vbmeta": [
>                  ["avbtool", "make_vbmeta_image", "--flags", "2",
>                      "--padding_size", "$FLASH_PAGESIZE",
>                      "--output", "/vbmeta.img"],
> -                ["heimdall", "flash", "--$PARTITION_VBMETA",
> "/vbmeta.img"], +                ["heimdall", "flash",
> "--$PARTITION_VBMETA", "/vbmeta.img", +                 "$REBOOT",
> "$RESUME"],
>                  ["rm", "-f", "/vbmeta.img"]],
>              "flash_lk2nd": [
>                  ["heimdall_wait_for_device.sh"],
> -                ["heimdall", "flash", "--$PARTITION_KERNEL",
> "$BOOT/lk2nd.img"]] +                ["heimdall", "flash",
> "--$PARTITION_KERNEL", "$BOOT/lk2nd.img", +                 "$REBOOT",
> "$RESUME"]]
>          },
>      },
>      "adb": {
>          "depends": ["android-tools"],
> +        "no_reboot_supported": False,
>          "actions": {
>              "list_devices": [["adb", "-P", "5038", "devices"]],
>              "sideload": [["echo", "< wait for any device >"],
> @@ -1026,6 +1037,7 @@ flashers = {
>      },
>      "uuu": {
>          "depends": ["nxp-mfgtools-uuu"],
> +        "no_reboot_supported": False,
>          "actions": {
>              "flash_rootfs": [
>                  # There's a bug(?) in uuu where it clobbers the path in the
> cmd @@ -1038,6 +1050,7 @@ flashers = {
>      "rkdeveloptool": {
>          "split": True,
>          "depends": ["rkdeveloptool"],
> +        "no_reboot_supported": False,
>          "actions": {
>              "list_devices": [["rkdeveloptool", "list"]],
>              "flash_rootfs": [
> @@ -1052,6 +1065,7 @@ flashers = {
>      },
>      "mtkclient": {
>          "depends": ["mtkclient"],
> +        "no_reboot_supported": False,
>          "actions": {
>              "flash_rootfs": [["mtk", "w", "$PARTITION_ROOTFS",
>                                "$IMAGE"]],
> diff --git a/pmb/flasher/run.py b/pmb/flasher/run.py
> index 7daa7376..f66d9e2e 100644
> --- a/pmb/flasher/run.py
> +++ b/pmb/flasher/run.py
> @@ -51,6 +51,19 @@ def run(args, action, flavor=None):
>                             " <https://wiki.postmarketos.org/wiki/"
>                             "Deviceinfo_reference>")
> 
> +    if (not args.reboot and "flash" not in action) or \
> +        (not args.reboot and not cfg["no_reboot_supported"]):
> +        raise RuntimeError("This action does not support "
> +                           "the '--no-reboot' argument.")
> +
> +    if (not args.resume and "flash" not in action) or \
> +        (not args.resume and not cfg["no_reboot_supported"]):
> +        raise RuntimeError("This action does not support "
> +                           "the '--resume' argument.")
> +
> +    if args.reboot and "reboot_command" in cfg and "flash" in action:
> +        cfg["actions"][action].append(cfg["reboot_command"])
> +
>      # Run the commands of each action
>      for command in cfg["actions"][action]:
>          # Variable replacement
> @@ -66,5 +79,7 @@ def run(args, action, flavor=None):
>                      check_partition_blacklist(args, key, value)
>                      command[i] = command[i].replace(key, value)
> 
> +        # Remove empty strings
> +        command = [x for x in command if x != '']
>          # Run the action
>          pmb.chroot.root(args, command, output="interactive")
> diff --git a/pmb/flasher/variables.py b/pmb/flasher/variables.py
> index 6fd5469a..3cba6685 100644
> --- a/pmb/flasher/variables.py
> +++ b/pmb/flasher/variables.py
> @@ -62,6 +62,14 @@ def variables(args, flavor, method):
>      if args.deviceinfo["append_dtb"] == "true":
>          _dtb = "-dtb"
> 
> +    _reboot = ""
> +    if not args.reboot:
> +        _reboot = "--no-reboot"
> +
> +    _resume = ""
> +    if args.resume:
> +        _resume = "--resume"
> +
>      vars = {
>          "$BOOT": "/mnt/rootfs_" + args.device + "/boot",
>          "$DTB": _dtb,
> @@ -80,7 +88,9 @@ def variables(args, flavor, method):
>                           "/var/lib/postmarketos-android-recovery-installer"
> "/pmos-" + args.device + ".zip",
>          "$UUU_SCRIPT": "/mnt/rootfs_" + args.deviceinfo["codename"] +
> -                       "/usr/share/uuu/flash_script.lst"
> +                       "/usr/share/uuu/flash_script.lst",
> +        "$REBOOT": _reboot,
> +        "$RESUME": _resume
>      }
> 
>      # Backwards compatibility with old mkinitfs (pma#660)
> diff --git a/pmb/parse/arguments.py b/pmb/parse/arguments.py
> index ffd8780c..92f2fbf1 100644
> --- a/pmb/parse/arguments.py
> +++ b/pmb/parse/arguments.py
> @@ -230,6 +230,9 @@ def arguments_flasher(subparser):
>                                 " target device")
>      ret.add_argument("--method", help="override flash method",
>                       dest="flash_method", default=None)
> +    ret.add_argument("--no-reboot", help="do not reboot after flashing",
> dest="reboot", action="store_false") +    ret.add_argument("--resume",
> help="perform another action after using --no-reboot", dest="resume",
> action="store_true") +
>      sub = ret.add_subparsers(dest="action_flasher")
>      sub.required = True

Re: Re: [PATCH pmbootstrap] flasher: add support for '--no-reboot' and '--resume' for heimdall-bootimg and fastboot

Details
Message ID
<93cb93f7-ed45-481a-b20b-ef1b54552e0c@gmail.com>
In-Reply-To
<5734701.DvuYhMxLoT@z3ntu.xyz> (view parent)
DKIM signature
missing
Download raw message
No it would not work as you have to add the resume argument if you want
to flash again without rebooting the device
(https://git.sr.ht/~grimler/Heimdall/tree/master/Linux/README#L212). I
think we could add support for the no-reboot argument to most (if not
all) flash methods to make it consistent but i only have fastboot and
heimdall devices.

Re: [PATCH pmbootstrap] flasher: add support for '--no-reboot' and '--resume' for heimdall-bootimg and fastboot

Details
Message ID
<4880540.31r3eYUQgx@z3ntu.xyz>
In-Reply-To
<93cb93f7-ed45-481a-b20b-ef1b54552e0c@gmail.com> (view parent)
DKIM signature
missing
Download raw message
On Samstag, 16. Dezember 2023 12:42:22 CET Andras Sebok wrote:
> No it would not work as you have to add the resume argument if you want
> to flash again without rebooting the device
> (https://git.sr.ht/~grimler/Heimdall/tree/master/Linux/README#L212). I
> think we could add support for the no-reboot argument to most (if not
> all) flash methods to make it consistent but i only have fastboot and
> heimdall devices.

Can we maybe always pass --resume, or somehow figure out automatically if we 
need to pass --resume or not?
Details
Message ID
<CYA8X0JL4DS3.RJ5SF9V06G41@pm-mail-aerc>
In-Reply-To
<170240028195.15140.12385625685092383241-0@git.sr.ht> (view parent)
DKIM signature
missing
Download raw message
Hi Andreas,

thank you very much for the patch!

Looking at Heimdall git log, apparently it was able to auto-resume at
one point but then it was removed in 2013 in ebbc3e7c:

    - Removed auto-resume functionality - Although this feature was definitely
      nice to have; I believe it may be responsible for flashing compatibility
      issues for a variety of devices.

So while it would be much nicer to have this solved in Heimdall again,
it is probably a lot of effort if possible at all and it makes sense to
add the arguments to pmbootstrap.

Regarding the implementation, I would suggest to do it differently. This
is special behavior only for heimdall, AFAIK all other flash methods
don't reboot the device after each flash command. So I would not add
this to other flash methods.

How about implementing it as follows:

1. Adding the --no-reboot and --resume in a new argument_group (see
arguments_install for an example), so it shows up as:

  $ pmbootstrap flasher -h
  usage: pmbootstrap flasher [-h] … 
  
  positional arguments:
    {boot,flash_kernel,flash_lk2nd,flash_rootfs,flash_vbmeta,flash_dtbo,sideload,list_flavors,list_devices}
      boot                boot a kernel once
      flash_kernel        flash a kernel
      …
  
  options:
    -h, --help            show this help message and exit
    --method FLASH_METHOD
                          override flash method
  
  heimdall options:
    With heimdall as flash method, the device automatically reboots after
    each flash command. Use --no-reboot and --resume for multiple flash
    actions without reboot.
    --no-reboot		don't automatically reboot after flashing
    --resume		resume flashing after using --no-reboot

2. checking in the code, that if --no-reboot or --resume is used, the
   flash method is really heimdall, and if it is not, raise an error.

3. Not adding no_reboot_supported / reboot_command to the other flash
   methods in the config, only modifying the heimdall flash method.

What do you think about this, would you like to submit a new version?

Best,
Oliver
Reply to thread Export thread (mbox)