~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
11 3

[PATCH pmbootstrap 1/3] Replace repeated device package name construction with function

Details
Message ID
<20230513120625.1259793-1-luca@z3ntu.xyz>
DKIM signature
missing
Download raw message
Patch: +19 -7
There's a bunch of places where we need the name of the device package,
which currently just appends args.device to the string "device-".

Instead let's introduce a helper for this, similar to get_kernel_package
and get_nonfree_packages with the difference that it returns a string
instead of a list of string. First because it's not optional and second
because quite a few usages of the name require a simple string, not a
list.
---
 pmb/chroot/other.py        |  2 +-
 pmb/install/__init__.py    |  1 +
 pmb/install/_install.py    | 14 ++++++++++++--
 test/test_build_package.py |  7 ++++---
 test/test_pkgrel_bump.py   |  2 +-
 5 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/pmb/chroot/other.py b/pmb/chroot/other.py
index 4af5029c..7302089e 100644
--- a/pmb/chroot/other.py
+++ b/pmb/chroot/other.py
@@ -20,7 +20,7 @@ def kernel_flavor_installed(args, suffix, autoinstall=True):
    """
    # Automatically install the selected kernel
    if autoinstall:
        packages = ([f"device-{args.device}"] +
        packages = ([pmb.install.get_device_package(args.device)] +
                    pmb.install.get_kernel_package(args, args.device))
        pmb.chroot.apk.install(args, packages, suffix)

diff --git a/pmb/install/__init__.py b/pmb/install/__init__.py
index d44288ce..4089afb2 100644
--- a/pmb/install/__init__.py
+++ b/pmb/install/__init__.py
@@ -1,6 +1,7 @@
# Copyright 2023 Oliver Smith
# SPDX-License-Identifier: GPL-3.0-or-later
from pmb.install._install import install
from pmb.install._install import get_device_package
from pmb.install._install import get_kernel_package
from pmb.install.partition import partition
from pmb.install.partition import partition_cgpt
diff --git a/pmb/install/_install.py b/pmb/install/_install.py
index 9b76a96a..7e809afb 100644
--- a/pmb/install/_install.py
+++ b/pmb/install/_install.py
@@ -78,6 +78,16 @@ def get_nonfree_packages(args, device):
    return ret


def get_device_package(device):
    """
    Get the device package.

    :param device: code name, e.g. "sony-amami"
    :returns: the name of the package, e.g. "device-sony-amami"
    """
    return f"device-{device}"


def get_kernel_package(args, device):
    """
    Get the device's kernel subpackage based on the user's choice in
@@ -863,7 +873,7 @@ def install_on_device_installer(args, step, steps):
    # Prepare the installer chroot
    logging.info(f"*** ({step}/{steps}) CREATE ON-DEVICE INSTALLER ROOTFS ***")
    step += 1
    packages = ([f"device-{args.device}",
    packages = ([get_device_package(args.device),
                 "postmarketos-ondev"] +
                get_kernel_package(args, args.device) +
                get_nonfree_packages(args, args.device))
@@ -956,7 +966,7 @@ def create_device_rootfs(args, step, steps):

    # Fill install_packages
    install_packages = (pmb.config.install_device_packages +
                        ["device-" + args.device])
                        [get_device_package(args.device)])
    if not args.install_base:
        install_packages = [p for p in install_packages
                            if p != "postmarketos-base"]
diff --git a/test/test_build_package.py b/test/test_build_package.py
index 0d18b793..b52b3faf 100644
--- a/test/test_build_package.py
+++ b/test/test_build_package.py
@@ -14,6 +14,7 @@ import pmb.build._package
import pmb.config
import pmb.config.init
import pmb.helpers.logging
import pmb.install


@pytest.fixture
@@ -393,7 +394,7 @@ def test_build_local_source_high_level(args, tmpdir):
    aports = tmpdir + "/aports"
    aport = aports + "/device/testing/device-" + args.device
    os.makedirs(aport)
    path_original = pmb.helpers.pmaports.find(args, f"device-{args.device}")
    path_original = pmb.helpers.pmaports.find(args, pmb.install.get_device_package(args.device))
    shutil.copy(f"{path_original}/deviceinfo", aport)

    # aports: Add modified hello-world aport (source="", uses $builddir)
@@ -451,9 +452,9 @@ def test_build_abuild_leftovers(args, tmpdir):
    # aports: Add deviceinfo (required by pmbootstrap to start)
    tmpdir = str(tmpdir)
    aports = f"{tmpdir}/aports"
    aport = f"{aports}/device/testing/device-{args.device}"
    aport = f"{aports}/device/testing/" + pmb.install.get_device_package(args.device)
    os.makedirs(aport)
    path_original = pmb.helpers.pmaports.find(args, f"device-{args.device}")
    path_original = pmb.helpers.pmaports.find(args, pmb.install.get_device_package(args.device))
    shutil.copy(f"{path_original}/deviceinfo", aport)

    # aports: Add modified hello-world aport (source="", uses $builddir)
diff --git a/test/test_pkgrel_bump.py b/test/test_pkgrel_bump.py
index 9fd4c735..257385f8 100644
--- a/test/test_pkgrel_bump.py
+++ b/test/test_pkgrel_bump.py
@@ -80,7 +80,7 @@ def setup_work(args, tmpdir):
    for folder in ["device/testing", "main"]:
        pmb.helpers.run.user(args, ["mkdir", "-p", args.aports, tmpdir +
                                    "/_aports/" + folder])
    path_original = pmb.helpers.pmaports.find(args, f"device-{args.device}")
    path_original = pmb.helpers.pmaports.find(args, pmb.install.get_device_package(args.device))
    pmb.helpers.run.user(args, ["cp", "-r", path_original,
                                f"{tmpdir}/_aports/device/testing"])
    for pkgname in ["testlib", "testapp", "testsubpkg"]:
-- 
2.40.1

[PATCH pmbootstrap 2/3] pmb.flasher.frontend: Use elif instead of repeated if

Details
Message ID
<20230513120625.1259793-2-luca@z3ntu.xyz>
In-Reply-To
<20230513120625.1259793-1-luca@z3ntu.xyz> (view parent)
DKIM signature
missing
Download raw message
Patch: +7 -7
Only one action can be provided through the args, so there's no point in
checking subsequent actions if a previous one has matched.
---
 pmb/flasher/frontend.py | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/pmb/flasher/frontend.py b/pmb/flasher/frontend.py
index a88986a4..08e26063 100644
--- a/pmb/flasher/frontend.py
+++ b/pmb/flasher/frontend.py
@@ -137,17 +137,17 @@ def frontend(args):

    if action in ["boot", "flash_kernel"]:
        kernel(args)
    if action == "flash_rootfs":
    elif action == "flash_rootfs":
        rootfs(args)
    if action == "flash_vbmeta":
    elif action == "flash_vbmeta":
        flash_vbmeta(args)
    if action == "flash_dtbo":
    elif action == "flash_dtbo":
        flash_dtbo(args)
    if action == "list_flavors":
    elif action == "list_flavors":
        list_flavors(args)
    if action == "list_devices":
    elif action == "list_devices":
        list_devices(args)
    if action == "sideload":
    elif action == "sideload":
        sideload(args)
    if action in ["flash_lk2nd"]:
    elif action in ["flash_lk2nd"]:
        flash_lk2nd(args)
-- 
2.40.1

[PATCH pmbootstrap 3/3] pmb.flasher: Improve flash_lk2nd action

Details
Message ID
<20230513120625.1259793-3-luca@z3ntu.xyz>
In-Reply-To
<20230513120625.1259793-1-luca@z3ntu.xyz> (view parent)
DKIM signature
missing
Download raw message
Patch: +23 -6
* Check that we're not already running lk2nd as flashing boot partition
  inside lk2nd is different to flashing boot partition outside. We could
  improve this in the future to use "flash lk2nd lk2nd.img" as
  documented in the file.
* Install the device package before flashing as that's expected to have
  a dependency on the correct lk2nd package.
* Remove some log message in unusual styles for pmbootstrap.
* Group flash_lk2nd action together with the other flash actions and use
  string comparison.

See also: https://gitlab.com/postmarketOS/pmaports/-/issues/2074
---
 pmb/flasher/frontend.py | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/pmb/flasher/frontend.py b/pmb/flasher/frontend.py
index 08e26063..8df17ea8 100644
--- a/pmb/flasher/frontend.py
+++ b/pmb/flasher/frontend.py
@@ -110,15 +110,32 @@ def sideload(args):


def flash_lk2nd(args):
    method = args.flash_method or args.deviceinfo["flash_method"]
    if method == "fastboot":
        # In the future this could be expanded to use "fastboot flash lk2nd $img"
        # which reflashes/updates lk2nd from itself. For now let the user handle this
        # manually since supporting the codepath with heimdall requires more effort.
        logging.info("(native) checking current fastboot product")
        pmb.flasher.init(args)
        output = pmb.chroot.root(args, ["fastboot", "getvar", "product"],
                                 output="interactive", output_return=True)
        # Variable "product" is e.g. "LK2ND_MSM8974" or "lk2nd-msm8226" depending
        # on the lk2nd version.
        if "lk2nd" in output.lower():
            raise RuntimeError("You are currently running lk2nd. Please reboot into the regular"
                               " bootloader mode to re-flash lk2nd.")

    # Install device package since that should also install lk2nd package
    suffix = "rootfs_" + args.device
    pmb.chroot.apk.install(args, [pmb.install.get_device_package(args.device)], suffix)

    chroot_path = args.work + "/chroot_rootfs_" + args.device
    lk2nd_path = "/boot/lk2nd.img"
    if not os.path.exists(chroot_path + lk2nd_path):
        raise RuntimeError(f"{chroot_path+lk2nd_path} doesn't exist. Your"
        raise RuntimeError(f"The file {chroot_path+lk2nd_path} does not exist. Your"
                           " device may not support lk2nd.")

    logging.info(lk2nd_path)
    logging.info("It's normal if fastboot warns"
                 " \"Image not signed or corrupt\" or a similar warning")
    logging.info("(native) flash lk2nd image")
    pmb.flasher.run(args, "flash_lk2nd")


@@ -143,11 +160,11 @@ def frontend(args):
        flash_vbmeta(args)
    elif action == "flash_dtbo":
        flash_dtbo(args)
    elif action == "flash_lk2nd":
        flash_lk2nd(args)
    elif action == "list_flavors":
        list_flavors(args)
    elif action == "list_devices":
        list_devices(args)
    elif action == "sideload":
        sideload(args)
    elif action in ["flash_lk2nd"]:
        flash_lk2nd(args)
-- 
2.40.1

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

builds.sr.ht <builds@sr.ht>
Details
Message ID
<CSL5ELH0IZL3.JKNDXASG52LB@cirno2>
In-Reply-To
<20230513120625.1259793-1-luca@z3ntu.xyz> (view parent)
DKIM signature
missing
Download raw message
pmbootstrap/patches/.build.yml: SUCCESS in 21m37s

[Replace repeated device package name construction with function][0] from [Luca Weiss][1]

[0]: https://lists.sr.ht/~postmarketos/pmbootstrap-devel/patches/41111
[1]: luca@z3ntu.xyz

✓ #989108 SUCCESS pmbootstrap/patches/.build.yml https://builds.sr.ht/~postmarketos/job/989108
Details
Message ID
<CSOUF9E78KWF.3PYT1S0M0S0EN@pm-mail-aerc>
In-Reply-To
<20230513120625.1259793-1-luca@z3ntu.xyz> (view parent)
DKIM signature
missing
Download raw message
On Sat May 13, 2023 at 2:06 PM CEST, Luca Weiss wrote:
> There's a bunch of places where we need the name of the device package,
> which currently just appends args.device to the string "device-".
>
> Instead let's introduce a helper for this, similar to get_kernel_package
> and get_nonfree_packages with the difference that it returns a string
> instead of a list of string. First because it's not optional and second
> because quite a few usages of the name require a simple string, not a
> list.

Sorry, but what is the advantage here? The existing code is shorter and
does the same. The get_nonfree_package and get_nonfree_packages
functions are more complex than just adding "device-" infront of the
device name. Maybe drop this patch?

> ---
>  pmb/chroot/other.py        |  2 +-
>  pmb/install/__init__.py    |  1 +
>  pmb/install/_install.py    | 14 ++++++++++++--
>  test/test_build_package.py |  7 ++++---
>  test/test_pkgrel_bump.py   |  2 +-
>  5 files changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/pmb/chroot/other.py b/pmb/chroot/other.py
> index 4af5029c..7302089e 100644
> --- a/pmb/chroot/other.py
> +++ b/pmb/chroot/other.py
> @@ -20,7 +20,7 @@ def kernel_flavor_installed(args, suffix, autoinstall=True):
>      """
>      # Automatically install the selected kernel
>      if autoinstall:
> -        packages = ([f"device-{args.device}"] +
> +        packages = ([pmb.install.get_device_package(args.device)] +
>                      pmb.install.get_kernel_package(args, args.device))
>          pmb.chroot.apk.install(args, packages, suffix)
>  
> diff --git a/pmb/install/__init__.py b/pmb/install/__init__.py
> index d44288ce..4089afb2 100644
> --- a/pmb/install/__init__.py
> +++ b/pmb/install/__init__.py
> @@ -1,6 +1,7 @@
>  # Copyright 2023 Oliver Smith
>  # SPDX-License-Identifier: GPL-3.0-or-later
>  from pmb.install._install import install
> +from pmb.install._install import get_device_package
>  from pmb.install._install import get_kernel_package
>  from pmb.install.partition import partition
>  from pmb.install.partition import partition_cgpt
> diff --git a/pmb/install/_install.py b/pmb/install/_install.py
> index 9b76a96a..7e809afb 100644
> --- a/pmb/install/_install.py
> +++ b/pmb/install/_install.py
> @@ -78,6 +78,16 @@ def get_nonfree_packages(args, device):
>      return ret
>  
>  
> +def get_device_package(device):
> +    """
> +    Get the device package.
> +
> +    :param device: code name, e.g. "sony-amami"
> +    :returns: the name of the package, e.g. "device-sony-amami"
> +    """
> +    return f"device-{device}"
> +
> +
>  def get_kernel_package(args, device):
>      """
>      Get the device's kernel subpackage based on the user's choice in
> @@ -863,7 +873,7 @@ def install_on_device_installer(args, step, steps):
>      # Prepare the installer chroot
>      logging.info(f"*** ({step}/{steps}) CREATE ON-DEVICE INSTALLER ROOTFS ***")
>      step += 1
> -    packages = ([f"device-{args.device}",
> +    packages = ([get_device_package(args.device),
>                   "postmarketos-ondev"] +
>                  get_kernel_package(args, args.device) +
>                  get_nonfree_packages(args, args.device))
> @@ -956,7 +966,7 @@ def create_device_rootfs(args, step, steps):
>  
>      # Fill install_packages
>      install_packages = (pmb.config.install_device_packages +
> -                        ["device-" + args.device])
> +                        [get_device_package(args.device)])
>      if not args.install_base:
>          install_packages = [p for p in install_packages
>                              if p != "postmarketos-base"]
> diff --git a/test/test_build_package.py b/test/test_build_package.py
> index 0d18b793..b52b3faf 100644
> --- a/test/test_build_package.py
> +++ b/test/test_build_package.py
> @@ -14,6 +14,7 @@ import pmb.build._package
>  import pmb.config
>  import pmb.config.init
>  import pmb.helpers.logging
> +import pmb.install
>  
>  
>  @pytest.fixture
> @@ -393,7 +394,7 @@ def test_build_local_source_high_level(args, tmpdir):
>      aports = tmpdir + "/aports"
>      aport = aports + "/device/testing/device-" + args.device
>      os.makedirs(aport)
> -    path_original = pmb.helpers.pmaports.find(args, f"device-{args.device}")
> +    path_original = pmb.helpers.pmaports.find(args, pmb.install.get_device_package(args.device))
>      shutil.copy(f"{path_original}/deviceinfo", aport)
>  
>      # aports: Add modified hello-world aport (source="", uses $builddir)
> @@ -451,9 +452,9 @@ def test_build_abuild_leftovers(args, tmpdir):
>      # aports: Add deviceinfo (required by pmbootstrap to start)
>      tmpdir = str(tmpdir)
>      aports = f"{tmpdir}/aports"
> -    aport = f"{aports}/device/testing/device-{args.device}"
> +    aport = f"{aports}/device/testing/" + pmb.install.get_device_package(args.device)
>      os.makedirs(aport)
> -    path_original = pmb.helpers.pmaports.find(args, f"device-{args.device}")
> +    path_original = pmb.helpers.pmaports.find(args, pmb.install.get_device_package(args.device))
>      shutil.copy(f"{path_original}/deviceinfo", aport)
>  
>      # aports: Add modified hello-world aport (source="", uses $builddir)
> diff --git a/test/test_pkgrel_bump.py b/test/test_pkgrel_bump.py
> index 9fd4c735..257385f8 100644
> --- a/test/test_pkgrel_bump.py
> +++ b/test/test_pkgrel_bump.py
> @@ -80,7 +80,7 @@ def setup_work(args, tmpdir):
>      for folder in ["device/testing", "main"]:
>          pmb.helpers.run.user(args, ["mkdir", "-p", args.aports, tmpdir +
>                                      "/_aports/" + folder])
> -    path_original = pmb.helpers.pmaports.find(args, f"device-{args.device}")
> +    path_original = pmb.helpers.pmaports.find(args, pmb.install.get_device_package(args.device))
>      pmb.helpers.run.user(args, ["cp", "-r", path_original,
>                                  f"{tmpdir}/_aports/device/testing"])
>      for pkgname in ["testlib", "testapp", "testsubpkg"]:
> -- 
> 2.40.1

Re: [PATCH pmbootstrap 2/3] pmb.flasher.frontend: Use elif instead of repeated if

Details
Message ID
<CSOUFGECGHLK.2T8BQF96I065K@pm-mail-aerc>
In-Reply-To
<20230513120625.1259793-2-luca@z3ntu.xyz> (view parent)
DKIM signature
missing
Download raw message
On Sat May 13, 2023 at 2:06 PM CEST, Luca Weiss wrote:
> Only one action can be provided through the args, so there's no point in
> checking subsequent actions if a previous one has matched.

Thanks!

Reviewed-by: Oliver Smith <ollieparanoid@postmarketos.org>

> ---
>  pmb/flasher/frontend.py | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/pmb/flasher/frontend.py b/pmb/flasher/frontend.py
> index a88986a4..08e26063 100644
> --- a/pmb/flasher/frontend.py
> +++ b/pmb/flasher/frontend.py
> @@ -137,17 +137,17 @@ def frontend(args):
>  
>      if action in ["boot", "flash_kernel"]:
>          kernel(args)
> -    if action == "flash_rootfs":
> +    elif action == "flash_rootfs":
>          rootfs(args)
> -    if action == "flash_vbmeta":
> +    elif action == "flash_vbmeta":
>          flash_vbmeta(args)
> -    if action == "flash_dtbo":
> +    elif action == "flash_dtbo":
>          flash_dtbo(args)
> -    if action == "list_flavors":
> +    elif action == "list_flavors":
>          list_flavors(args)
> -    if action == "list_devices":
> +    elif action == "list_devices":
>          list_devices(args)
> -    if action == "sideload":
> +    elif action == "sideload":
>          sideload(args)
> -    if action in ["flash_lk2nd"]:
> +    elif action in ["flash_lk2nd"]:
>          flash_lk2nd(args)
> -- 
> 2.40.1

Re: [PATCH pmbootstrap 3/3] pmb.flasher: Improve flash_lk2nd action

Details
Message ID
<CSOUMH3SXA9O.19TKV5HRBRWE4@pm-mail-aerc>
In-Reply-To
<20230513120625.1259793-3-luca@z3ntu.xyz> (view parent)
DKIM signature
missing
Download raw message
Thanks for the patch!

On Sat May 13, 2023 at 2:06 PM CEST, Luca Weiss wrote:
> * Check that we're not already running lk2nd as flashing boot partition
>   inside lk2nd is different to flashing boot partition outside. We could
>   improve this in the future to use "flash lk2nd lk2nd.img" as
>   documented in the file.
> * Install the device package before flashing as that's expected to have
>   a dependency on the correct lk2nd package.
> * Remove some log message in unusual styles for pmbootstrap.
> * Group flash_lk2nd action together with the other flash actions and use
>   string comparison.

Sounds like these are different logical changes and it would make sense
to put them in separate commits.

>
> See also: https://gitlab.com/postmarketOS/pmaports/-/issues/2074
> ---
>  pmb/flasher/frontend.py | 29 +++++++++++++++++++++++------
>  1 file changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/pmb/flasher/frontend.py b/pmb/flasher/frontend.py
> index 08e26063..8df17ea8 100644
> --- a/pmb/flasher/frontend.py
> +++ b/pmb/flasher/frontend.py
> @@ -110,15 +110,32 @@ def sideload(args):
>  
>  
>  def flash_lk2nd(args):
> +    method = args.flash_method or args.deviceinfo["flash_method"]
> +    if method == "fastboot":
> +        # In the future this could be expanded to use "fastboot flash lk2nd $img"
> +        # which reflashes/updates lk2nd from itself. For now let the user handle this
> +        # manually since supporting the codepath with heimdall requires more effort.
> +        logging.info("(native) checking current fastboot product")
> +        pmb.flasher.init(args)
> +        output = pmb.chroot.root(args, ["fastboot", "getvar", "product"],
> +                                 output="interactive", output_return=True)
> +        # Variable "product" is e.g. "LK2ND_MSM8974" or "lk2nd-msm8226" depending
> +        # on the lk2nd version.
> +        if "lk2nd" in output.lower():
> +            raise RuntimeError("You are currently running lk2nd. Please reboot into the regular"
> +                               " bootloader mode to re-flash lk2nd.")
> +
> +    # Install device package since that should also install lk2nd package
> +    suffix = "rootfs_" + args.device
> +    pmb.chroot.apk.install(args, [pmb.install.get_device_package(args.device)], suffix)

I wonder if it makes sense to figure out the lk2nd package name from the
device package, and only install that instead of installing the entire
device package. It would be a bit faster, what do you think about that?

> +
>      chroot_path = args.work + "/chroot_rootfs_" + args.device
>      lk2nd_path = "/boot/lk2nd.img"
>      if not os.path.exists(chroot_path + lk2nd_path):
> -        raise RuntimeError(f"{chroot_path+lk2nd_path} doesn't exist. Your"
> +        raise RuntimeError(f"The file {chroot_path+lk2nd_path} does not exist. Your"
>                             " device may not support lk2nd.")

If we try to figure out the lk2nd package name before installing the
device package, we could give this feedback directly.

>  
> -    logging.info(lk2nd_path)
> -    logging.info("It's normal if fastboot warns"
> -                 " \"Image not signed or corrupt\" or a similar warning")

Agreed that these logs are not in the style of usual pmb log messages.
But the information about "Image not signed or corrupt", does this still
appear? We could print it with a NOTE: infront to have it more in the
style of other pmb messages.

> +    logging.info("(native) flash lk2nd image")
>      pmb.flasher.run(args, "flash_lk2nd")
>  
>  
> @@ -143,11 +160,11 @@ def frontend(args):
>          flash_vbmeta(args)
>      elif action == "flash_dtbo":
>          flash_dtbo(args)
> +    elif action == "flash_lk2nd":
> +        flash_lk2nd(args)
>      elif action == "list_flavors":
>          list_flavors(args)
>      elif action == "list_devices":
>          list_devices(args)
>      elif action == "sideload":
>          sideload(args)
> -    elif action in ["flash_lk2nd"]:
> -        flash_lk2nd(args)
> -- 
> 2.40.1
Details
Message ID
<168435696489.4934.9207254471701819474.b4-ty@postmarketos.org>
In-Reply-To
<20230513120625.1259793-1-luca@z3ntu.xyz> (view parent)
DKIM signature
missing
Download raw message
On Sat, 13 May 2023 14:06:23 +0200, Luca Weiss wrote:
> There's a bunch of places where we need the name of the device package,
> which currently just appends args.device to the string "device-".
> 
> Instead let's introduce a helper for this, similar to get_kernel_package
> and get_nonfree_packages with the difference that it returns a string
> instead of a list of string. First because it's not optional and second
> because quite a few usages of the name require a simple string, not a
> list.
> 
> [...]

Applied the 2nd patch, thanks!

[2/3] pmb.flasher.frontend: Use elif instead of repeated if
      commit: 071dc99f685c7472993af0e3ea8e3ad304c20993

Best regards,
-- 
Oliver Smith <ollieparanoid@postmarketos.org>

Re: [PATCH pmbootstrap 1/3] Replace repeated device package name construction with function

Details
Message ID
<8260433.NyiUUSuA9g@z3ntu.xyz>
In-Reply-To
<CSOUF9E78KWF.3PYT1S0M0S0EN@pm-mail-aerc> (view parent)
DKIM signature
missing
Download raw message
On Mittwoch, 17. Mai 2023 22:42:30 CEST Oliver Smith wrote:
> On Sat May 13, 2023 at 2:06 PM CEST, Luca Weiss wrote:
> > There's a bunch of places where we need the name of the device package,
> > which currently just appends args.device to the string "device-".
> > 
> > Instead let's introduce a helper for this, similar to get_kernel_package
> > and get_nonfree_packages with the difference that it returns a string
> > instead of a list of string. First because it's not optional and second
> > because quite a few usages of the name require a simple string, not a
> > list.
> 
> Sorry, but what is the advantage here? The existing code is shorter and
> does the same. The get_nonfree_package and get_nonfree_packages
> functions are more complex than just adding "device-" infront of the
> device name. Maybe drop this patch?

I'm not really a fan of having this kind of magic string concatenation spread 
all over, and instead like putting something like this in functions where it's 
easy to see where it's used.

But obviously it's a very simple append here so if you want to keep it like it 
was, I'm fine with dropping this one and doing the same in the 3/3 patch.

Regards
Luca

> 
> > ---
> > 
> >  pmb/chroot/other.py        |  2 +-
> >  pmb/install/__init__.py    |  1 +
> >  pmb/install/_install.py    | 14 ++++++++++++--
> >  test/test_build_package.py |  7 ++++---
> >  test/test_pkgrel_bump.py   |  2 +-
> >  5 files changed, 19 insertions(+), 7 deletions(-)
> > 
> > diff --git a/pmb/chroot/other.py b/pmb/chroot/other.py
> > index 4af5029c..7302089e 100644
> > --- a/pmb/chroot/other.py
> > +++ b/pmb/chroot/other.py
> > 
> > @@ -20,7 +20,7 @@ def kernel_flavor_installed(args, suffix, 
autoinstall=True):
> >      """
> >      # Automatically install the selected kernel
> > 
> >      if autoinstall:
> > -        packages = ([f"device-{args.device}"] +
> > +        packages = ([pmb.install.get_device_package(args.device)] +
> > 
> >                      pmb.install.get_kernel_package(args, args.device))
> >          
> >          pmb.chroot.apk.install(args, packages, suffix)
> > 
> > diff --git a/pmb/install/__init__.py b/pmb/install/__init__.py
> > index d44288ce..4089afb2 100644
> > --- a/pmb/install/__init__.py
> > +++ b/pmb/install/__init__.py
> > @@ -1,6 +1,7 @@
> > 
> >  # Copyright 2023 Oliver Smith
> >  # SPDX-License-Identifier: GPL-3.0-or-later
> >  from pmb.install._install import install
> > 
> > +from pmb.install._install import get_device_package
> > 
> >  from pmb.install._install import get_kernel_package
> >  from pmb.install.partition import partition
> >  from pmb.install.partition import partition_cgpt
> > 
> > diff --git a/pmb/install/_install.py b/pmb/install/_install.py
> > index 9b76a96a..7e809afb 100644
> > --- a/pmb/install/_install.py
> > +++ b/pmb/install/_install.py
> > 
> > @@ -78,6 +78,16 @@ def get_nonfree_packages(args, device):
> >      return ret
> > 
> > +def get_device_package(device):
> > +    """
> > +    Get the device package.
> > +
> > +    :param device: code name, e.g. "sony-amami"
> > +    :returns: the name of the package, e.g. "device-sony-amami"
> > +    """
> > +    return f"device-{device}"
> > +
> > +
> > 
> >  def get_kernel_package(args, device):
> >      """
> >      Get the device's kernel subpackage based on the user's choice in
> > 
> > @@ -863,7 +873,7 @@ def install_on_device_installer(args, step, steps):
> >      # Prepare the installer chroot
> >      logging.info(f"*** ({step}/{steps}) CREATE ON-DEVICE INSTALLER ROOTFS
> >      ***")
> >      step += 1
> > 
> > -    packages = ([f"device-{args.device}",
> > +    packages = ([get_device_package(args.device),
> > 
> >                   "postmarketos-ondev"] +
> >                  
> >                  get_kernel_package(args, args.device) +
> >                  get_nonfree_packages(args, args.device))
> > 
> > @@ -956,7 +966,7 @@ def create_device_rootfs(args, step, steps):
> >      # Fill install_packages
> >      install_packages = (pmb.config.install_device_packages +
> > 
> > -                        ["device-" + args.device])
> > +                        [get_device_package(args.device)])
> > 
> >      if not args.install_base:
> >          install_packages = [p for p in install_packages
> >          
> >                              if p != "postmarketos-base"]
> > 
> > diff --git a/test/test_build_package.py b/test/test_build_package.py
> > index 0d18b793..b52b3faf 100644
> > --- a/test/test_build_package.py
> > +++ b/test/test_build_package.py
> > @@ -14,6 +14,7 @@ import pmb.build._package
> > 
> >  import pmb.config
> >  import pmb.config.init
> >  import pmb.helpers.logging
> > 
> > +import pmb.install
> > 
> >  @pytest.fixture
> > 
> > @@ -393,7 +394,7 @@ def test_build_local_source_high_level(args, tmpdir):
> >      aports = tmpdir + "/aports"
> >      aport = aports + "/device/testing/device-" + args.device
> >      os.makedirs(aport)
> > 
> > -    path_original = pmb.helpers.pmaports.find(args,
> > f"device-{args.device}") +    path_original =
> > pmb.helpers.pmaports.find(args,
> > pmb.install.get_device_package(args.device))> 
> >      shutil.copy(f"{path_original}/deviceinfo", aport)
> >      
> >      # aports: Add modified hello-world aport (source="", uses $builddir)
> > 
> > @@ -451,9 +452,9 @@ def test_build_abuild_leftovers(args, tmpdir):
> >      # aports: Add deviceinfo (required by pmbootstrap to start)
> >      tmpdir = str(tmpdir)
> >      aports = f"{tmpdir}/aports"
> > 
> > -    aport = f"{aports}/device/testing/device-{args.device}"
> > +    aport = f"{aports}/device/testing/" +
> > pmb.install.get_device_package(args.device)> 
> >      os.makedirs(aport)
> > 
> > -    path_original = pmb.helpers.pmaports.find(args,
> > f"device-{args.device}") +    path_original =
> > pmb.helpers.pmaports.find(args,
> > pmb.install.get_device_package(args.device))> 
> >      shutil.copy(f"{path_original}/deviceinfo", aport)
> >      
> >      # aports: Add modified hello-world aport (source="", uses $builddir)
> > 
> > diff --git a/test/test_pkgrel_bump.py b/test/test_pkgrel_bump.py
> > index 9fd4c735..257385f8 100644
> > --- a/test/test_pkgrel_bump.py
> > +++ b/test/test_pkgrel_bump.py
> > 
> > @@ -80,7 +80,7 @@ def setup_work(args, tmpdir):
> >      for folder in ["device/testing", "main"]:
> >          pmb.helpers.run.user(args, ["mkdir", "-p", args.aports, tmpdir +
> >          
> >                                      "/_aports/" + folder])
> > 
> > -    path_original = pmb.helpers.pmaports.find(args,
> > f"device-{args.device}") +    path_original =
> > pmb.helpers.pmaports.find(args,
> > pmb.install.get_device_package(args.device))> 
> >      pmb.helpers.run.user(args, ["cp", "-r", path_original,
> >      
> >                                  f"{tmpdir}/_aports/device/testing"])
> >      
> >      for pkgname in ["testlib", "testapp", "testsubpkg"]:

Re: [PATCH pmbootstrap 3/3] pmb.flasher: Improve flash_lk2nd action

Details
Message ID
<2150390.Mh6RI2rZIc@z3ntu.xyz>
In-Reply-To
<CSOUMH3SXA9O.19TKV5HRBRWE4@pm-mail-aerc> (view parent)
DKIM signature
missing
Download raw message
On Mittwoch, 17. Mai 2023 22:51:55 CEST Oliver Smith wrote:
> Thanks for the patch!
> 
> On Sat May 13, 2023 at 2:06 PM CEST, Luca Weiss wrote:
> > * Check that we're not already running lk2nd as flashing boot partition
> > 
> >   inside lk2nd is different to flashing boot partition outside. We could
> >   improve this in the future to use "flash lk2nd lk2nd.img" as
> >   documented in the file.
> > 
> > * Install the device package before flashing as that's expected to have
> > 
> >   a dependency on the correct lk2nd package.
> > 
> > * Remove some log message in unusual styles for pmbootstrap.
> > * Group flash_lk2nd action together with the other flash actions and use
> > 
> >   string comparison.
> 
> Sounds like these are different logical changes and it would make sense
> to put them in separate commits.

Since it's only a small diff in the end, do you really want 4 commits for 
this? If you don't mind, I'd keep it one patch.

> 
> > See also: https://gitlab.com/postmarketOS/pmaports/-/issues/2074
> > ---
> > 
> >  pmb/flasher/frontend.py | 29 +++++++++++++++++++++++------
> >  1 file changed, 23 insertions(+), 6 deletions(-)
> > 
> > diff --git a/pmb/flasher/frontend.py b/pmb/flasher/frontend.py
> > index 08e26063..8df17ea8 100644
> > --- a/pmb/flasher/frontend.py
> > +++ b/pmb/flasher/frontend.py
> > 
> > @@ -110,15 +110,32 @@ def sideload(args):
> >  def flash_lk2nd(args):
> > +    method = args.flash_method or args.deviceinfo["flash_method"]
> > +    if method == "fastboot":
> > +        # In the future this could be expanded to use "fastboot flash
> > lk2nd $img" +        # which reflashes/updates lk2nd from itself. For now
> > let the user handle this +        # manually since supporting the
> > codepath with heimdall requires more effort. +       
> > logging.info("(native) checking current fastboot product") +       
> > pmb.flasher.init(args)
> > +        output = pmb.chroot.root(args, ["fastboot", "getvar", "product"],
> > +                                 output="interactive",
> > output_return=True)
> > +        # Variable "product" is e.g. "LK2ND_MSM8974" or "lk2nd-msm8226"
> > depending +        # on the lk2nd version.
> > +        if "lk2nd" in output.lower():
> > +            raise RuntimeError("You are currently running lk2nd. Please
> > reboot into the regular" +                               " bootloader
> > mode to re-flash lk2nd.") +
> > +    # Install device package since that should also install lk2nd package
> > +    suffix = "rootfs_" + args.device
> > +    pmb.chroot.apk.install(args,
> > [pmb.install.get_device_package(args.device)], suffix)
> I wonder if it makes sense to figure out the lk2nd package name from the
> device package, and only install that instead of installing the entire
> device package. It would be a bit faster, what do you think about that?

see below

> 
> > +
> > 
> >      chroot_path = args.work + "/chroot_rootfs_" + args.device
> >      lk2nd_path = "/boot/lk2nd.img"
> > 
> >      if not os.path.exists(chroot_path + lk2nd_path):
> > -        raise RuntimeError(f"{chroot_path+lk2nd_path} doesn't exist.
> > Your"
> > +        raise RuntimeError(f"The file {chroot_path+lk2nd_path} does not
> > exist. Your"> 
> >                             " device may not support lk2nd.")
> 
> If we try to figure out the lk2nd package name before installing the
> device package, we could give this feedback directly.

You're thinking about something like parsing the device APKBUILD, getting 
depends from it and filtering anything containing lk2nd and then manually 
installing it (adding it to world)? Feels a bit overkill compared to just 
installing the device package..

> 
> > -    logging.info(lk2nd_path)
> > -    logging.info("It's normal if fastboot warns"
> > -                 " \"Image not signed or corrupt\" or a similar warning")
> 
> Agreed that these logs are not in the style of usual pmb log messages.
> But the information about "Image not signed or corrupt", does this still
> appear? We could print it with a NOTE: infront to have it more in the
> style of other pmb messages.

If this happens, then this should be printed when flashing any image with 
fastboot, pretty sure it's not unique to lk2nd. For now I'm fine with just 
removing it and re-adding it in the future if necessary.

Regards
Luca

> 
> > +    logging.info("(native) flash lk2nd image")
> > 
> >      pmb.flasher.run(args, "flash_lk2nd")
> > 
> > @@ -143,11 +160,11 @@ def frontend(args):
> >          flash_vbmeta(args)
> >      
> >      elif action == "flash_dtbo":
> >          flash_dtbo(args)
> > 
> > +    elif action == "flash_lk2nd":
> > +        flash_lk2nd(args)
> > 
> >      elif action == "list_flavors":
> >          list_flavors(args)
> >      
> >      elif action == "list_devices":
> >          list_devices(args)
> >      
> >      elif action == "sideload":
> >          sideload(args)
> > 
> > -    elif action in ["flash_lk2nd"]:
> > -        flash_lk2nd(args)

Re: [PATCH pmbootstrap 3/3] pmb.flasher: Improve flash_lk2nd action

Details
Message ID
<CT3QH7EB3HIB.2DYW590ZNO5AA@pm-mail-aerc>
In-Reply-To
<2150390.Mh6RI2rZIc@z3ntu.xyz> (view parent)
DKIM signature
missing
Download raw message
On Tue May 23, 2023 at 6:10 PM CEST, Luca Weiss wrote:
> On Mittwoch, 17. Mai 2023 22:51:55 CEST Oliver Smith wrote:
> > Thanks for the patch!
> > 
> > On Sat May 13, 2023 at 2:06 PM CEST, Luca Weiss wrote:
> > > * Check that we're not already running lk2nd as flashing boot partition
> > > 
> > >   inside lk2nd is different to flashing boot partition outside. We could
> > >   improve this in the future to use "flash lk2nd lk2nd.img" as
> > >   documented in the file.
> > > 
> > > * Install the device package before flashing as that's expected to have
> > > 
> > >   a dependency on the correct lk2nd package.
> > > 
> > > * Remove some log message in unusual styles for pmbootstrap.
> > > * Group flash_lk2nd action together with the other flash actions and use
> > > 
> > >   string comparison.
> > 
> > Sounds like these are different logical changes and it would make sense
> > to put them in separate commits.
>
> Since it's only a small diff in the end, do you really want 4 commits for 
> this? If you don't mind, I'd keep it one patch.

I'd prefer it, but it's not that important. You can keep it in one
patch.

>
> > 
> > > See also: https://gitlab.com/postmarketOS/pmaports/-/issues/2074
> > > ---
> > > 
> > >  pmb/flasher/frontend.py | 29 +++++++++++++++++++++++------
> > >  1 file changed, 23 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/pmb/flasher/frontend.py b/pmb/flasher/frontend.py
> > > index 08e26063..8df17ea8 100644
> > > --- a/pmb/flasher/frontend.py
> > > +++ b/pmb/flasher/frontend.py
> > > 
> > > @@ -110,15 +110,32 @@ def sideload(args):
> > >  def flash_lk2nd(args):
> > > +    method = args.flash_method or args.deviceinfo["flash_method"]
> > > +    if method == "fastboot":
> > > +        # In the future this could be expanded to use "fastboot flash
> > > lk2nd $img" +        # which reflashes/updates lk2nd from itself. For now
> > > let the user handle this +        # manually since supporting the
> > > codepath with heimdall requires more effort. +       
> > > logging.info("(native) checking current fastboot product") +       
> > > pmb.flasher.init(args)
> > > +        output = pmb.chroot.root(args, ["fastboot", "getvar", "product"],
> > > +                                 output="interactive",
> > > output_return=True)
> > > +        # Variable "product" is e.g. "LK2ND_MSM8974" or "lk2nd-msm8226"
> > > depending +        # on the lk2nd version.
> > > +        if "lk2nd" in output.lower():
> > > +            raise RuntimeError("You are currently running lk2nd. Please
> > > reboot into the regular" +                               " bootloader
> > > mode to re-flash lk2nd.") +
> > > +    # Install device package since that should also install lk2nd package
> > > +    suffix = "rootfs_" + args.device
> > > +    pmb.chroot.apk.install(args,
> > > [pmb.install.get_device_package(args.device)], suffix)
> > I wonder if it makes sense to figure out the lk2nd package name from the
> > device package, and only install that instead of installing the entire
> > device package. It would be a bit faster, what do you think about that?
>
> see below
>
> > 
> > > +
> > > 
> > >      chroot_path = args.work + "/chroot_rootfs_" + args.device
> > >      lk2nd_path = "/boot/lk2nd.img"
> > > 
> > >      if not os.path.exists(chroot_path + lk2nd_path):
> > > -        raise RuntimeError(f"{chroot_path+lk2nd_path} doesn't exist.
> > > Your"
> > > +        raise RuntimeError(f"The file {chroot_path+lk2nd_path} does not
> > > exist. Your"> 
> > >                             " device may not support lk2nd.")
> > 
> > If we try to figure out the lk2nd package name before installing the
> > device package, we could give this feedback directly.
>
> You're thinking about something like parsing the device APKBUILD, getting 
> depends from it and filtering anything containing lk2nd and then manually 
> installing it (adding it to world)? Feels a bit overkill compared to just 
> installing the device package..

It sounds like a lot, but in code it's just this and makes the user
experience better as mentioned, we can directly say if lk2nd dep is
missing:

    # Get the lk2nd package (which is a dependency of the device package)
    device_pkg = f"device-{args.device}"
    apkbuild = pmb.helpers.pmaports.get(args, device_pkg)
    lk2nd_pkg = None
    for dep in apkbuild["depends"]:
        if dep.startswith("lk2nd"):
            lk2nd_pkg = dep
            break

    if not lk2nd_pkg:
        raise RuntimeError(f"{device_pkg} does not depend on any lk2nd package")

>
> > 
> > > -    logging.info(lk2nd_path)
> > > -    logging.info("It's normal if fastboot warns"
> > > -                 " \"Image not signed or corrupt\" or a similar warning")
> > 
> > Agreed that these logs are not in the style of usual pmb log messages.
> > But the information about "Image not signed or corrupt", does this still
> > appear? We could print it with a NOTE: infront to have it more in the
> > style of other pmb messages.
>
> If this happens, then this should be printed when flashing any image with 
> fastboot, pretty sure it's not unique to lk2nd. For now I'm fine with just 
> removing it and re-adding it in the future if necessary.

Okay, fine with me too.

>
> Regards
> Luca
>
> > 
> > > +    logging.info("(native) flash lk2nd image")
> > > 
> > >      pmb.flasher.run(args, "flash_lk2nd")
> > > 
> > > @@ -143,11 +160,11 @@ def frontend(args):
> > >          flash_vbmeta(args)
> > >      
> > >      elif action == "flash_dtbo":
> > >          flash_dtbo(args)
> > > 
> > > +    elif action == "flash_lk2nd":
> > > +        flash_lk2nd(args)
> > > 
> > >      elif action == "list_flavors":
> > >          list_flavors(args)
> > >      
> > >      elif action == "list_devices":
> > >          list_devices(args)
> > >      
> > >      elif action == "sideload":
> > >          sideload(args)
> > > 
> > > -    elif action in ["flash_lk2nd"]:
> > > -        flash_lk2nd(args)
Details
Message ID
<CT3QI7OBVR8Z.3BS9W3SG54BHC@pm-mail-aerc>
In-Reply-To
<8260433.NyiUUSuA9g@z3ntu.xyz> (view parent)
DKIM signature
missing
Download raw message
On Tue May 23, 2023 at 5:27 PM CEST, Luca Weiss wrote:
> On Mittwoch, 17. Mai 2023 22:42:30 CEST Oliver Smith wrote:
> > On Sat May 13, 2023 at 2:06 PM CEST, Luca Weiss wrote:
> > > There's a bunch of places where we need the name of the device package,
> > > which currently just appends args.device to the string "device-".
> > > 
> > > Instead let's introduce a helper for this, similar to get_kernel_package
> > > and get_nonfree_packages with the difference that it returns a string
> > > instead of a list of string. First because it's not optional and second
> > > because quite a few usages of the name require a simple string, not a
> > > list.
> > 
> > Sorry, but what is the advantage here? The existing code is shorter and
> > does the same. The get_nonfree_package and get_nonfree_packages
> > functions are more complex than just adding "device-" infront of the
> > device name. Maybe drop this patch?
>
> I'm not really a fan of having this kind of magic string concatenation spread 
> all over, and instead like putting something like this in functions where it's 
> easy to see where it's used.
>
> But obviously it's a very simple append here so if you want to keep it like it 
> was, I'm fine with dropping this one and doing the same in the 3/3 patch.

Yes I'd rather drop this patch. Thanks!

>
> Regards
> Luca

>
> > 
> > > ---
> > > 
> > >  pmb/chroot/other.py        |  2 +-
> > >  pmb/install/__init__.py    |  1 +
> > >  pmb/install/_install.py    | 14 ++++++++++++--
> > >  test/test_build_package.py |  7 ++++---
> > >  test/test_pkgrel_bump.py   |  2 +-
> > >  5 files changed, 19 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/pmb/chroot/other.py b/pmb/chroot/other.py
> > > index 4af5029c..7302089e 100644
> > > --- a/pmb/chroot/other.py
> > > +++ b/pmb/chroot/other.py
> > > 
> > > @@ -20,7 +20,7 @@ def kernel_flavor_installed(args, suffix, 
> autoinstall=True):
> > >      """
> > >      # Automatically install the selected kernel
> > > 
> > >      if autoinstall:
> > > -        packages = ([f"device-{args.device}"] +
> > > +        packages = ([pmb.install.get_device_package(args.device)] +
> > > 
> > >                      pmb.install.get_kernel_package(args, args.device))
> > >          
> > >          pmb.chroot.apk.install(args, packages, suffix)
> > > 
> > > diff --git a/pmb/install/__init__.py b/pmb/install/__init__.py
> > > index d44288ce..4089afb2 100644
> > > --- a/pmb/install/__init__.py
> > > +++ b/pmb/install/__init__.py
> > > @@ -1,6 +1,7 @@
> > > 
> > >  # Copyright 2023 Oliver Smith
> > >  # SPDX-License-Identifier: GPL-3.0-or-later
> > >  from pmb.install._install import install
> > > 
> > > +from pmb.install._install import get_device_package
> > > 
> > >  from pmb.install._install import get_kernel_package
> > >  from pmb.install.partition import partition
> > >  from pmb.install.partition import partition_cgpt
> > > 
> > > diff --git a/pmb/install/_install.py b/pmb/install/_install.py
> > > index 9b76a96a..7e809afb 100644
> > > --- a/pmb/install/_install.py
> > > +++ b/pmb/install/_install.py
> > > 
> > > @@ -78,6 +78,16 @@ def get_nonfree_packages(args, device):
> > >      return ret
> > > 
> > > +def get_device_package(device):
> > > +    """
> > > +    Get the device package.
> > > +
> > > +    :param device: code name, e.g. "sony-amami"
> > > +    :returns: the name of the package, e.g. "device-sony-amami"
> > > +    """
> > > +    return f"device-{device}"
> > > +
> > > +
> > > 
> > >  def get_kernel_package(args, device):
> > >      """
> > >      Get the device's kernel subpackage based on the user's choice in
> > > 
> > > @@ -863,7 +873,7 @@ def install_on_device_installer(args, step, steps):
> > >      # Prepare the installer chroot
> > >      logging.info(f"*** ({step}/{steps}) CREATE ON-DEVICE INSTALLER ROOTFS
> > >      ***")
> > >      step += 1
> > > 
> > > -    packages = ([f"device-{args.device}",
> > > +    packages = ([get_device_package(args.device),
> > > 
> > >                   "postmarketos-ondev"] +
> > >                  
> > >                  get_kernel_package(args, args.device) +
> > >                  get_nonfree_packages(args, args.device))
> > > 
> > > @@ -956,7 +966,7 @@ def create_device_rootfs(args, step, steps):
> > >      # Fill install_packages
> > >      install_packages = (pmb.config.install_device_packages +
> > > 
> > > -                        ["device-" + args.device])
> > > +                        [get_device_package(args.device)])
> > > 
> > >      if not args.install_base:
> > >          install_packages = [p for p in install_packages
> > >          
> > >                              if p != "postmarketos-base"]
> > > 
> > > diff --git a/test/test_build_package.py b/test/test_build_package.py
> > > index 0d18b793..b52b3faf 100644
> > > --- a/test/test_build_package.py
> > > +++ b/test/test_build_package.py
> > > @@ -14,6 +14,7 @@ import pmb.build._package
> > > 
> > >  import pmb.config
> > >  import pmb.config.init
> > >  import pmb.helpers.logging
> > > 
> > > +import pmb.install
> > > 
> > >  @pytest.fixture
> > > 
> > > @@ -393,7 +394,7 @@ def test_build_local_source_high_level(args, tmpdir):
> > >      aports = tmpdir + "/aports"
> > >      aport = aports + "/device/testing/device-" + args.device
> > >      os.makedirs(aport)
> > > 
> > > -    path_original = pmb.helpers.pmaports.find(args,
> > > f"device-{args.device}") +    path_original =
> > > pmb.helpers.pmaports.find(args,
> > > pmb.install.get_device_package(args.device))> 
> > >      shutil.copy(f"{path_original}/deviceinfo", aport)
> > >      
> > >      # aports: Add modified hello-world aport (source="", uses $builddir)
> > > 
> > > @@ -451,9 +452,9 @@ def test_build_abuild_leftovers(args, tmpdir):
> > >      # aports: Add deviceinfo (required by pmbootstrap to start)
> > >      tmpdir = str(tmpdir)
> > >      aports = f"{tmpdir}/aports"
> > > 
> > > -    aport = f"{aports}/device/testing/device-{args.device}"
> > > +    aport = f"{aports}/device/testing/" +
> > > pmb.install.get_device_package(args.device)> 
> > >      os.makedirs(aport)
> > > 
> > > -    path_original = pmb.helpers.pmaports.find(args,
> > > f"device-{args.device}") +    path_original =
> > > pmb.helpers.pmaports.find(args,
> > > pmb.install.get_device_package(args.device))> 
> > >      shutil.copy(f"{path_original}/deviceinfo", aport)
> > >      
> > >      # aports: Add modified hello-world aport (source="", uses $builddir)
> > > 
> > > diff --git a/test/test_pkgrel_bump.py b/test/test_pkgrel_bump.py
> > > index 9fd4c735..257385f8 100644
> > > --- a/test/test_pkgrel_bump.py
> > > +++ b/test/test_pkgrel_bump.py
> > > 
> > > @@ -80,7 +80,7 @@ def setup_work(args, tmpdir):
> > >      for folder in ["device/testing", "main"]:
> > >          pmb.helpers.run.user(args, ["mkdir", "-p", args.aports, tmpdir +
> > >          
> > >                                      "/_aports/" + folder])
> > > 
> > > -    path_original = pmb.helpers.pmaports.find(args,
> > > f"device-{args.device}") +    path_original =
> > > pmb.helpers.pmaports.find(args,
> > > pmb.install.get_device_package(args.device))> 
> > >      pmb.helpers.run.user(args, ["cp", "-r", path_original,
> > >      
> > >                                  f"{tmpdir}/_aports/device/testing"])
> > >      
> > >      for pkgname in ["testlib", "testapp", "testsubpkg"]:
Reply to thread Export thread (mbox)