Make sure the user has at least 256 MiB set as their installation size,
refuse to start the installation otherwise. The default was changed in
2021, 03e9fb05 ("pmb.config.init.boot_size: set to 256 MiB (MR 2037)").

If the user ran "pmbootstrap init" before that commit, the pmbootstrap
config will have the old default set. It is very annoying when you do an
installation with it and only realize it when you run into errors, e.g.
while upgrading. I had that when testing the upgrade to the v23.12
release and also adjusted postmarketos-release-upgrade to warn if the
boot partition is smaller than expected.
---
 pmb/install/_install.py | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/pmb/install/_install.py b/pmb/install/_install.py
index 1adfe789..b48dc84d 100644
--- a/pmb/install/_install.py
+++ b/pmb/install/_install.py
@@ -5,6 +5,7 @@ import os
 import re
 import glob
 import shlex
+import sys
 
 import pmb.chroot
 import pmb.chroot.apk
@@ -621,6 +622,19 @@ def write_cgpt_kpart(args, layout, suffix):
         args, ["dd", f"if={filename}", f"of=/dev/installp{layout['kernel']}"])
 
 
+def sanity_check_boot_size(args):
+    default = pmb.config.defaults["boot_size"]
+    if int(args.boot_size) >= int(default):
+        return
+    logging.error("ERROR: your pmbootstrap has a small boot_size of"
+                  f" {args.boot_size} configured, probably because the config"
+                  " has been created with an old version.")
+    logging.error("This can lead to problems later on, we recommend setting it"
+                  f" to {default} MiB.")
+    logging.error(f"Run 'pmbootstrap config boot_size {default}' and try again.")
+    sys.exit(1)
+
+
 def sanity_check_disk(args):
     device = args.disk
     device_name = os.path.basename(device)
@@ -1154,6 +1168,7 @@ def create_device_rootfs(args, step, steps):
 
 def install(args):
     # Sanity checks
+    sanity_check_boot_size(args)
     if not args.android_recovery_zip and args.disk:
         sanity_check_disk(args)
         sanity_check_disk_size(args)
-- 
2.43.0

pmbootstrap/patches/.build.yml: SUCCESS in 18m25s

[install: throw error if boot_size is too small][0] from [Oliver Smith][1]

[0]: https://lists.sr.ht/~postmarketos/pmbootstrap-devel/patches/47511
[1]: ollieparanoid@postmarketos.org

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

On Freitag, 8. Dezember 2023 23:27:08 CET Oliver Smith wrote:
> Make sure the user has at least 256 MiB set as their installation size,
> refuse to start the installation otherwise. The default was changed in
> 2021, 03e9fb05 ("pmb.config.init.boot_size: set to 256 MiB (MR 2037)").
> 
> If the user ran "pmbootstrap init" before that commit, the pmbootstrap
> config will have the old default set. It is very annoying when you do an
> installation with it and only realize it when you run into errors, e.g.
> while upgrading. I had that when testing the upgrade to the v23.12
> release and also adjusted postmarketos-release-upgrade to warn if the
> boot partition is smaller than expected.

Seems also my local config has 128MB still.

  $ pmbootstrap config boot_size
  128

Are we sure there's no random use case for old devices where someone wants to 
use a smaller boot partition?

If you think not:

Reviewed-by: Luca Weiss <luca@z3ntu.xyz>

> ---
>  pmb/install/_install.py | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/pmb/install/_install.py b/pmb/install/_install.py
> index 1adfe789..b48dc84d 100644
> --- a/pmb/install/_install.py
> +++ b/pmb/install/_install.py
> @@ -5,6 +5,7 @@ import os
>  import re
>  import glob
>  import shlex
> +import sys
> 
>  import pmb.chroot
>  import pmb.chroot.apk
> @@ -621,6 +622,19 @@ def write_cgpt_kpart(args, layout, suffix):
>          args, ["dd", f"if={filename}",
> f"of=/dev/installp{layout['kernel']}"])
> 
> 
> +def sanity_check_boot_size(args):
> +    default = pmb.config.defaults["boot_size"]
> +    if int(args.boot_size) >= int(default):
> +        return
> +    logging.error("ERROR: your pmbootstrap has a small boot_size of"
> +                  f" {args.boot_size} configured, probably because the
> config" +                  " has been created with an old version.")
> +    logging.error("This can lead to problems later on, we recommend setting
> it" +                  f" to {default} MiB.")
> +    logging.error(f"Run 'pmbootstrap config boot_size {default}' and try
> again.") +    sys.exit(1)
> +
> +
>  def sanity_check_disk(args):
>      device = args.disk
>      device_name = os.path.basename(device)
> @@ -1154,6 +1168,7 @@ def create_device_rootfs(args, step, steps):
> 
>  def install(args):
>      # Sanity checks
> +    sanity_check_boot_size(args)
>      if not args.android_recovery_zip and args.disk:
>          sanity_check_disk(args)
>          sanity_check_disk_size(args)

On Sat Dec 16, 2023 at 3:50 PM CET, Luca Weiss wrote:
> On Freitag, 8. Dezember 2023 23:27:08 CET Oliver Smith wrote:
> > Make sure the user has at least 256 MiB set as their installation size,
> > refuse to start the installation otherwise. The default was changed in
> > 2021, 03e9fb05 ("pmb.config.init.boot_size: set to 256 MiB (MR 2037)").
> > 
> > If the user ran "pmbootstrap init" before that commit, the pmbootstrap
> > config will have the old default set. It is very annoying when you do an
> > installation with it and only realize it when you run into errors, e.g.
> > while upgrading. I had that when testing the upgrade to the v23.12
> > release and also adjusted postmarketos-release-upgrade to warn if the
> > boot partition is smaller than expected.
>
> Seems also my local config has 128MB still.
>
>   $ pmbootstrap config boot_size
>   128

I'm not surprised :D

>
> Are we sure there's no random use case for old devices where someone wants to 
> use a smaller boot partition?

There might be. If that is the case, people will complain and we can
re-iterate on this patch (maybe add a deviceinfo option for min.
recommended boot partition size, and let it default to 256 MiB?).

On the other hand, there for sure are still installs that have the old
default configured, and may run into problems because of it. So I think
adding this patch worth it.

>
> If you think not:
>
> Reviewed-by: Luca Weiss <luca@z3ntu.xyz>

Thanks for the review!

>
> > ---
> >  pmb/install/_install.py | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/pmb/install/_install.py b/pmb/install/_install.py
> > index 1adfe789..b48dc84d 100644
> > --- a/pmb/install/_install.py
> > +++ b/pmb/install/_install.py
> > @@ -5,6 +5,7 @@ import os
> >  import re
> >  import glob
> >  import shlex
> > +import sys
> > 
> >  import pmb.chroot
> >  import pmb.chroot.apk
> > @@ -621,6 +622,19 @@ def write_cgpt_kpart(args, layout, suffix):
> >          args, ["dd", f"if={filename}",
> > f"of=/dev/installp{layout['kernel']}"])
> > 
> > 
> > +def sanity_check_boot_size(args):
> > +    default = pmb.config.defaults["boot_size"]
> > +    if int(args.boot_size) >= int(default):
> > +        return
> > +    logging.error("ERROR: your pmbootstrap has a small boot_size of"
> > +                  f" {args.boot_size} configured, probably because the
> > config" +                  " has been created with an old version.")
> > +    logging.error("This can lead to problems later on, we recommend setting
> > it" +                  f" to {default} MiB.")
> > +    logging.error(f"Run 'pmbootstrap config boot_size {default}' and try
> > again.") +    sys.exit(1)
> > +
> > +
> >  def sanity_check_disk(args):
> >      device = args.disk
> >      device_name = os.path.basename(device)
> > @@ -1154,6 +1168,7 @@ def create_device_rootfs(args, step, steps):
> > 
> >  def install(args):
> >      # Sanity checks
> > +    sanity_check_boot_size(args)
> >      if not args.android_recovery_zip and args.disk:
> >          sanity_check_disk(args)
> >          sanity_check_disk_size(args)

On Fri, 8 Dec 2023 23:27:08 +0100, Oliver Smith wrote:
> Make sure the user has at least 256 MiB set as their installation size,
> refuse to start the installation otherwise. The default was changed in
> 2021, 03e9fb05 ("pmb.config.init.boot_size: set to 256 MiB (MR 2037)").
> 
> If the user ran "pmbootstrap init" before that commit, the pmbootstrap
> config will have the old default set. It is very annoying when you do an
> installation with it and only realize it when you run into errors, e.g.
> while upgrading. I had that when testing the upgrade to the v23.12
> release and also adjusted postmarketos-release-upgrade to warn if the
> boot partition is smaller than expected.
> 
> [...]

Applied, thanks!

[1/1] install: throw error if boot_size is too small
      commit: e223fd03eb408e292aa065109b705692e376a008

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