[bug#55663] [PATCH] Image API: add FAT32 support

[bug#55663] [PATCH] Image API: add FAT32 support

[Pavel Shlyak]

[-- Attachment #1: Type: text/plain, Size: 275 bytes --]

I believe it is an important change as Raspberry PI (and I suppose some other boards) cannot boot with fat16 boot partitions. 
With this patch, "vfat" is treated as fat16 partition not to break backward-compatibility. "fat32", on the other hand, creates a fat32 partition.

[-- Attachment #2: 0001-Image-API-support-FAT32-partitions.patch --]
[-- Type: application/octet-stream, Size: 2470 bytes --]

From 0700c443d9162ac4928ac8fee58278c1f0824736 Mon Sep 17 00:00:00 2001
From: Pavel Shlyak <p.shlyak@pantherx.org>
Date: Thu, 26 May 2022 21:00:51 +0300
Subject: [PATCH] Image API: support FAT32 partitions

---
 gnu/build/image.scm  | 8 +++++---
 gnu/system/image.scm | 2 ++
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/gnu/build/image.scm b/gnu/build/image.scm
index 3e8b94e2d6..804685c7fb 100644
--- a/gnu/build/image.scm
+++ b/gnu/build/image.scm
@@ -95,12 +95,12 @@ (define* (make-ext-image partition target root
                            (estimate-partition-size root)
                            size)))))))
 
-(define* (make-vfat-image partition target root)
+(define* (make-vfat-image partition target root fs_bits)
   "Handle the creation of VFAT partition images.  See 'make-partition-image'."
   (let ((size (partition-size partition))
         (label (partition-label partition)))
     (invoke "fakeroot" "mkdosfs" "-n" label "-C" target
-            "-F" "16" "-S" "1024"
+            "-F" fs_bits "-S" "1024"
             (size-in-kib
              (if (eq? size 'guess)
                  (estimate-partition-size root)
@@ -121,7 +121,9 @@ (define* (make-partition-image partition-sexp target root)
      ((string-prefix? "ext" type)
       (make-ext-image partition target root))
      ((string=? type "vfat")
-      (make-vfat-image partition target root))
+      (make-vfat-image partition target root "16"))
+     ((string=? type "fat32")
+      (make-vfat-image partition target root "32"))
      (else
       (raise (condition
               (&message
diff --git a/gnu/system/image.scm b/gnu/system/image.scm
index dd32e58c2d..0dd16ed34e 100644
--- a/gnu/system/image.scm
+++ b/gnu/system/image.scm
@@ -313,6 +313,7 @@ (define (partition->dos-type partition)
          ((member 'esp flags) "0xEF")
          ((string-prefix? "ext" file-system) "0x83")
          ((string=? file-system "vfat") "0x0E")
+         ((string=? file-system "fat32") "0x0C")
          (else
           (raise (condition
                   (&message
@@ -330,6 +331,7 @@ (define (partition->gpt-type partition)
          ((member 'esp flags) "U")
          ((string-prefix? "ext" file-system) "L")
          ((string=? file-system "vfat") "F")
+         ((string=? file-system "fat32") "F")
          (else
           (raise (condition
                   (&message
-- 
2.32.1 (Apple Git-133)

[bug#55663] [PATCH] Image API: add FAT32 support

[Tobias Geerinckx-Rice via Guix-patches via]

[-- Attachment #1: Type: text/plain, Size: 500 bytes --]

Hi Pavel,

Pavel Shlyak 写道：
> I believe it is an important change as Raspberry PI (and I 
> suppose some other boards) cannot boot with fat16 boot 
> partitions. 

LGTM in principle.

The FS_BITS argument should be a regular number.  You can coerce 
it with number->string in the command line.

On the subject of being explicit is good: we should add "fat16" 
type and treat "vfat" as a softly deprecated alias.  But that can 
be done in a later patch.

Kind regards,

T G-R

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 247 bytes --]

[bug#55663] [PATCH] Image API: add FAT32 support

[Pavel Shlyak]

[-- Attachment #1: Type: text/plain, Size: 113 bytes --]

Thank you for a quick response!
I have changed it according to your recommendations. I hope it’s better now.

[-- Attachment #2: 0001-Image-API-support-FAT32-partitions.patch --]
[-- Type: application/octet-stream, Size: 2815 bytes --]

From 8128aa174f76646eedbf94c3f03cf1cff0f304d0 Mon Sep 17 00:00:00 2001
From: Pavel Shlyak <p.shlyak@pantherx.org>
Date: Thu, 26 May 2022 21:00:51 +0300
Subject: [PATCH] Image API: support FAT32 partitions

---
 gnu/build/image.scm  | 10 ++++++----
 gnu/system/image.scm |  9 +++++++--
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/gnu/build/image.scm b/gnu/build/image.scm
index 3e8b94e2d6..d1398144a5 100644
--- a/gnu/build/image.scm
+++ b/gnu/build/image.scm
@@ -95,12 +95,12 @@ (define* (make-ext-image partition target root
                            (estimate-partition-size root)
                            size)))))))
 
-(define* (make-vfat-image partition target root)
+(define* (make-vfat-image partition target root fs_bits)
   "Handle the creation of VFAT partition images.  See 'make-partition-image'."
   (let ((size (partition-size partition))
         (label (partition-label partition)))
     (invoke "fakeroot" "mkdosfs" "-n" label "-C" target
-            "-F" "16" "-S" "1024"
+            "-F" (number->string fs_bits) "-S" "1024"
             (size-in-kib
              (if (eq? size 'guess)
                  (estimate-partition-size root)
@@ -120,8 +120,10 @@ (define* (make-partition-image partition-sexp target root)
     (cond
      ((string-prefix? "ext" type)
       (make-ext-image partition target root))
-     ((string=? type "vfat")
-      (make-vfat-image partition target root))
+     ((or (string=? type "vfat") (string=? type "fat16"))
+      (make-vfat-image partition target root 16))
+     ((string=? type "fat32")
+      (make-vfat-image partition target root 32))
      (else
       (raise (condition
               (&message
diff --git a/gnu/system/image.scm b/gnu/system/image.scm
index dd32e58c2d..7e45f750d9 100644
--- a/gnu/system/image.scm
+++ b/gnu/system/image.scm
@@ -312,7 +312,8 @@ (define (partition->dos-type partition)
         (cond
          ((member 'esp flags) "0xEF")
          ((string-prefix? "ext" file-system) "0x83")
-         ((string=? file-system "vfat") "0x0E")
+         ((or (string=? file-system "vfat") (string=? file-system "fat16")) "0x0E")
+         ((string=? file-system "fat32") "0x0C")
          (else
           (raise (condition
                   (&message
@@ -329,7 +330,11 @@ (define (partition->gpt-type partition)
         (cond
          ((member 'esp flags) "U")
          ((string-prefix? "ext" file-system) "L")
-         ((string=? file-system "vfat") "F")
+         (
+          (or (string=? file-system "vfat")
+              (string=? file-system "fat16")
+              (string=? file-system "fat32")
+              ) "F")
          (else
           (raise (condition
                   (&message
-- 
2.32.1 (Apple Git-133)


[-- Attachment #3: Type: text/plain, Size: 613 bytes --]



> 26 мая 2022 г., в 21:07, Tobias Geerinckx-Rice <me@tobias.gr> написал(а):
> 
> Hi Pavel,
> 
> Pavel Shlyak 写道：
>> I believe it is an important change as Raspberry PI (and I suppose some other boards) cannot boot with fat16 boot partitions. 
> 
> LGTM in principle.
> 
> The FS_BITS argument should be a regular number.  You can coerce it with number->string in the command line.
> 
> On the subject of being explicit is good: we should add "fat16" type and treat "vfat" as a softly deprecated alias.  But that can be done in a later patch.
> 
> Kind regards,
> 
> T G-R

[bug#55663] [PATCH] Image API: add FAT32 support

[Pavel Shlyak]

Please, do not merge that until further notice. It looks like there’s a problem with the code.

[bug#55663] [PATCH] Image API: add FAT32 support

[Pavel Shlyak]

[-- Attachment #1: Type: text/plain, Size: 254 bytes --]

I have updated the patch so now it automatically sets block size for non-esp partitions. The previous behavior (for esp) was retained not to break things I’m not quite aware of.
Now I finally got correct/bootable partition layout with Raspberry PI4.

[-- Attachment #2: 0001-Image-API-support-FAT32-partitions.patch --]
[-- Type: application/octet-stream, Size: 3421 bytes --]

From 9049891a4fe9e9f4d66c78526fcdac9d5f0531bc Mon Sep 17 00:00:00 2001
From: Pavel Shlyak <p.shlyak@pantherx.org>
Date: Thu, 26 May 2022 21:00:51 +0300
Subject: [PATCH] Image API: support FAT32 partitions

---
 gnu/build/image.scm  | 24 ++++++++++++++----------
 gnu/system/image.scm |  9 +++++++--
 2 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/gnu/build/image.scm b/gnu/build/image.scm
index 3e8b94e2d6..692fd1f28a 100644
--- a/gnu/build/image.scm
+++ b/gnu/build/image.scm
@@ -95,16 +95,18 @@ (define* (make-ext-image partition target root
                            (estimate-partition-size root)
                            size)))))))
 
-(define* (make-vfat-image partition target root)
+(define* (make-vfat-image partition target root fs_bits)
   "Handle the creation of VFAT partition images.  See 'make-partition-image'."
   (let ((size (partition-size partition))
-        (label (partition-label partition)))
-    (invoke "fakeroot" "mkdosfs" "-n" label "-C" target
-            "-F" "16" "-S" "1024"
-            (size-in-kib
-             (if (eq? size 'guess)
-                 (estimate-partition-size root)
-                 size)))
+        (label (partition-label partition))
+        (flags (partition-flags partition)))
+    (apply invoke "fakeroot" "mkdosfs" "-n" label "-C" target
+                          "-F" (number->string fs_bits)
+                          (size-in-kib
+                           (if (eq? size 'guess)
+                               (estimate-partition-size root)
+                               size))
+                    (if (member 'esp flags) (list "-S" "1024") '()))
     (for-each (lambda (file)
                 (unless (member file '("." ".."))
                   (invoke "mcopy" "-bsp" "-i" target
@@ -120,8 +122,10 @@ (define* (make-partition-image partition-sexp target root)
     (cond
      ((string-prefix? "ext" type)
       (make-ext-image partition target root))
-     ((string=? type "vfat")
-      (make-vfat-image partition target root))
+     ((or (string=? type "vfat") (string=? type "fat16"))
+      (make-vfat-image partition target root 16))
+     ((string=? type "fat32")
+      (make-vfat-image partition target root 32))
      (else
       (raise (condition
               (&message
diff --git a/gnu/system/image.scm b/gnu/system/image.scm
index dd32e58c2d..7e45f750d9 100644
--- a/gnu/system/image.scm
+++ b/gnu/system/image.scm
@@ -312,7 +312,8 @@ (define (partition->dos-type partition)
         (cond
          ((member 'esp flags) "0xEF")
          ((string-prefix? "ext" file-system) "0x83")
-         ((string=? file-system "vfat") "0x0E")
+         ((or (string=? file-system "vfat") (string=? file-system "fat16")) "0x0E")
+         ((string=? file-system "fat32") "0x0C")
          (else
           (raise (condition
                   (&message
@@ -329,7 +330,11 @@ (define (partition->gpt-type partition)
         (cond
          ((member 'esp flags) "U")
          ((string-prefix? "ext" file-system) "L")
-         ((string=? file-system "vfat") "F")
+         (
+          (or (string=? file-system "vfat")
+              (string=? file-system "fat16")
+              (string=? file-system "fat32")
+              ) "F")
          (else
           (raise (condition
                   (&message
-- 
2.32.1 (Apple Git-133)

[bug#55663] [PATCH] Image API: add FAT32 support

[Mathieu Othacehe]

Hello Pavel,

>  gnu/build/image.scm  | 24 ++++++++++++++----------
>  gnu/system/image.scm |  9 +++++++--

Please write a commit message following the guidelines available here:
https://guix.gnu.org/manual/en/html_node/Submitting-Patches.html

> +(define* (make-vfat-image partition target root fs_bits)

s/fs_bits/fs-bits/

> +         ((or (string=? file-system "vfat") (string=? file-system "fat16")) "0x0E")

This line is longer than 78 characters you can break it between the two
strings comparisons.

> +         (
> +          (or (string=? file-system "vfat")

Merge those two lines.

> +              (string=? file-system "fat32")
> +              ) "F")

Ditto.

Can you please send a v2?

Thanks for your contribution,

Mathieu

[bug#55663] [PATCH] Image API: add FAT32 support

[Pavel Shlyak]

[-- Attachment #1: 0001-guix-Image-API-support-FAT32-partitions.patch --]
[-- Type: application/octet-stream, Size: 3800 bytes --]

From 5ff7fcf80196111ab8d5f87a1d159ed0349153b4 Mon Sep 17 00:00:00 2001
From: Pavel Shlyak <p.shlyak@pantherx.org>
Date: Thu, 26 May 2022 21:00:51 +0300
Subject: [PATCH] guix: Image API: support FAT32 partitions

* gnu/build/image.scm (make-vfat-image): pass fs bits as argument and force 1kb logical sector size only if "ESP" flag is set.
  (make-partition-image): add "fat32" partition type, support explicit "fat16" type with vfat alias.
* gnu/system/image.scm (partition->dos-type partition): return file system IDs for "fat16" and "fat32" partitions.
  (partition->gpt-type partition): ditto.
---
 gnu/build/image.scm  | 24 ++++++++++++++----------
 gnu/system/image.scm |  8 ++++++--
 2 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/gnu/build/image.scm b/gnu/build/image.scm
index 3e8b94e2d6..ddfd34c111 100644
--- a/gnu/build/image.scm
+++ b/gnu/build/image.scm
@@ -95,16 +95,18 @@ (define* (make-ext-image partition target root
                            (estimate-partition-size root)
                            size)))))))
 
-(define* (make-vfat-image partition target root)
+(define* (make-vfat-image partition target root fs-bits)
   "Handle the creation of VFAT partition images.  See 'make-partition-image'."
   (let ((size (partition-size partition))
-        (label (partition-label partition)))
-    (invoke "fakeroot" "mkdosfs" "-n" label "-C" target
-            "-F" "16" "-S" "1024"
-            (size-in-kib
-             (if (eq? size 'guess)
-                 (estimate-partition-size root)
-                 size)))
+        (label (partition-label partition))
+        (flags (partition-flags partition)))
+    (apply invoke "fakeroot" "mkdosfs" "-n" label "-C" target
+                          "-F" (number->string fs-bits)
+                          (size-in-kib
+                           (if (eq? size 'guess)
+                               (estimate-partition-size root)
+                               size))
+                    (if (member 'esp flags) (list "-S" "1024") '()))
     (for-each (lambda (file)
                 (unless (member file '("." ".."))
                   (invoke "mcopy" "-bsp" "-i" target
@@ -120,8 +122,10 @@ (define* (make-partition-image partition-sexp target root)
     (cond
      ((string-prefix? "ext" type)
       (make-ext-image partition target root))
-     ((string=? type "vfat")
-      (make-vfat-image partition target root))
+     ((or (string=? type "vfat") (string=? type "fat16"))
+      (make-vfat-image partition target root 16))
+     ((string=? type "fat32")
+      (make-vfat-image partition target root 32))
      (else
       (raise (condition
               (&message
diff --git a/gnu/system/image.scm b/gnu/system/image.scm
index dd32e58c2d..f02f6e0b8c 100644
--- a/gnu/system/image.scm
+++ b/gnu/system/image.scm
@@ -312,7 +312,9 @@ (define (partition->dos-type partition)
         (cond
          ((member 'esp flags) "0xEF")
          ((string-prefix? "ext" file-system) "0x83")
-         ((string=? file-system "vfat") "0x0E")
+         ((or (string=? file-system "vfat")
+              (string=? file-system "fat16")) "0x0E")
+         ((string=? file-system "fat32") "0x0C")
          (else
           (raise (condition
                   (&message
@@ -329,7 +331,9 @@ (define (partition->gpt-type partition)
         (cond
          ((member 'esp flags) "U")
          ((string-prefix? "ext" file-system) "L")
-         ((string=? file-system "vfat") "F")
+         ((or (string=? file-system "vfat")
+              (string=? file-system "fat16")
+              (string=? file-system "fat32")) "F")
          (else
           (raise (condition
                   (&message
-- 
2.32.1 (Apple Git-133)


[-- Attachment #2: Type: text/plain, Size: 930 bytes --]



> 30 мая 2022 г., в 10:01, Mathieu Othacehe <othacehe@gnu.org> написал(а):
> 
> 
> Hello Pavel,
> 
>> gnu/build/image.scm  | 24 ++++++++++++++----------
>> gnu/system/image.scm |  9 +++++++--
> 
> Please write a commit message following the guidelines available here:
> https://guix.gnu.org/manual/en/html_node/Submitting-Patches.html
> 
>> +(define* (make-vfat-image partition target root fs_bits)
> 
> s/fs_bits/fs-bits/
> 
>> +         ((or (string=? file-system "vfat") (string=? file-system "fat16")) "0x0E")
> 
> This line is longer than 78 characters you can break it between the two
> strings comparisons.
> 
>> +         (
>> +          (or (string=? file-system "vfat")
> 
> Merge those two lines.
> 
>> +              (string=? file-system "fat32")
>> +              ) "F")
> 
> Ditto.
> 
> Can you please send a v2?
> 
> Thanks for your contribution,
> 
> Mathieu

bug#55663: [PATCH] Image API: add FAT32 support

[Mathieu Othacehe]

Hello Pavel,

Thanks for the v2, pushed on master.

Mathieu