GNU bug report logs - #55663
[PATCH] Image API: add FAT32 support

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 55663 in the body.
You can then email your comments to 55663 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Pavel Shlyak <p.shlyak <at> pantherx.org>
To: guix-patches <at> gnu.org
Subject: [PATCH] Image API: add FAT32 support
Date: Thu, 26 May 2022 21:02:22 +0300

[Message part 1 (text/plain, inline)]

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.

[0001-Image-API-support-FAT32-partitions.patch (application/octet-stream, attachment)]

Message #8 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Tobias Geerinckx-Rice <me <at> tobias.gr>
To: Pavel Shlyak <p.shlyak <at> pantherx.org>
Cc: guix-patches <at> gnu.org, 55663 <at> debbugs.gnu.org
Subject: Re: [bug#55663] [PATCH] Image API: add FAT32 support
Date: Thu, 26 May 2022 20:07:56 +0200

[Message part 1 (text/plain, inline)]

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

[signature.asc (application/pgp-signature, inline)]

Message #14 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Pavel Shlyak <p.shlyak <at> pantherx.org>
To: Tobias Geerinckx-Rice <me <at> tobias.gr>
Cc: guix-patches <at> gnu.org, 55663 <at> debbugs.gnu.org
Subject: Re: [bug#55663] [PATCH] Image API: add FAT32 support
Date: Thu, 26 May 2022 23:54:45 +0300

[Message part 1 (text/plain, inline)]

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

[0001-Image-API-support-FAT32-partitions.patch (application/octet-stream, attachment)]

[Message part 3 (text/plain, inline)]

> 26 мая 2022 г., в 21:07, Tobias Geerinckx-Rice <me <at> 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

Message #20 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Pavel Shlyak <p.shlyak <at> pantherx.org>
To: Tobias Geerinckx-Rice <me <at> tobias.gr>
Cc: guix-patches <at> gnu.org, 55663 <at> debbugs.gnu.org
Subject: Re: [bug#55663] [PATCH] Image API: add FAT32 support
Date: Sun, 29 May 2022 14:46:27 +0300

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

Message #26 received at 55663 <at> debbugs.gnu.org (full text, mbox):

From: Pavel Shlyak <p.shlyak <at> pantherx.org>
To: 55663 <at> debbugs.gnu.org
Subject: [PATCH] Image API: add FAT32 support
Date: Sun, 29 May 2022 17:35:50 +0300

[Message part 1 (text/plain, inline)]

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.

[0001-Image-API-support-FAT32-partitions.patch (application/octet-stream, attachment)]

Message #29 received at 55663 <at> debbugs.gnu.org (full text, mbox):

From: Mathieu Othacehe <othacehe <at> gnu.org>
To: Pavel Shlyak <p.shlyak <at> pantherx.org>
Cc: 55663 <at> debbugs.gnu.org
Subject: Re: bug#55663: [PATCH] Image API: add FAT32 support
Date: Mon, 30 May 2022 09:01:18 +0200

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

Message #32 received at 55663 <at> debbugs.gnu.org (full text, mbox):

From: Pavel Shlyak <p.shlyak <at> pantherx.org>
To: Mathieu Othacehe <othacehe <at> gnu.org>
Cc: 55663 <at> debbugs.gnu.org
Subject: Re: bug#55663: [PATCH] Image API: add FAT32 support
Date: Mon, 30 May 2022 12:01:27 +0300

[0001-guix-Image-API-support-FAT32-partitions.patch (application/octet-stream, attachment)]

[Message part 2 (text/plain, inline)]

> 30 мая 2022 г., в 10:01, Mathieu Othacehe <othacehe <at> 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

Message #37 received at 55663-done <at> debbugs.gnu.org (full text, mbox):

From: Mathieu Othacehe <othacehe <at> gnu.org>
To: Pavel Shlyak <p.shlyak <at> pantherx.org>
Cc: 55663-done <at> debbugs.gnu.org
Subject: Re: bug#55663: [PATCH] Image API: add FAT32 support
Date: Tue, 31 May 2022 15:07:44 +0200

Hello Pavel,

Thanks for the v2, pushed on master.

Mathieu

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.