Ubiquity crashes with mount points that are FAT16/32

The attachment "Patch for partman-basicfilesystems" seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

Apologies, it appears the filename path is missing from the patch. The file is fstab.d/basic

Unsubscribing the Ubuntu Sponsors Team from this bug report as there is nothing to sponsor; this patch needs to be reviewed and turned into a debdiff for that to happen.

Thank you for your work on this!

@Simon, I disagree with that, there is a patch and it's useful to have sponsors visiblity on it, I'm subscribing the team back, it's not because a patch isn't a debdiff including the packaging that it can't be reviewed/sponsored

@adamsmith I simplified your patch down to a 1-liner, can you verify it still fixes it for you? I have test builds in case it helps here:
https://launchpad.net/~ddstreet/+archive/ubuntu/lp1770093

@waveform since you're the pi man, do you have an opinion on this? For quick reference, this is the patch to the 'fstab.d/basic' file:

@@ -86,7 +86,7 @@
      fi
     fi
     # base-passwd defines gid 46 as group plugdev
- if [ "$method" != efi ]; then
+ if [ "$method" != efi -a "$mountpoint" != "/boot/firmware" ]; then
      options="$options,umask=007,gid=46"
     fi
     echo "$path" "$mountpoint" vfat $options 0 1

removing ubuntu-sponsors while I try to get feedback and sponsor this.

I'm not sure we should be encouraging people to try the installer on the Pi (for which we provide pre-installed images instead); still I don't see any particular harm in adding support for this to ubiquity for those that want to play with it.

The only thing that slightly concerns me with the patch is whether any *other* platforms use /boot/firmware for a non-FAT boot partition, and hence whether the test should be refined to be more Pi-specific? Testing for a "Raspberry Pi" prefix in /proc/device-tree/model would do the trick if that's desired?

Ack, good point thanks - I updated the patch to:

@@ -87,7 +87,9 @@
     fi
     # base-passwd defines gid 46 as group plugdev
     if [ "$method" != efi ]; then
- options="$options,umask=007,gid=46"
+ if ! ([ "$mountpoint" = "/boot/firmware" ] && grep -sq "Raspberry Pi" /proc/device-tree/model); then
+ options="$options,umask=007,gid=46"
+ fi
     fi
     echo "$path" "$mountpoint" vfat $options 0 1
    fi

@adamsmith can you test with this patch to make sure it fixes the install problem? I have builds of partman-basicfilesystems in this ppa:
https://launchpad.net/~ddstreet/+archive/ubuntu/lp1770093

Sorry for the slow reply. Launchpad bug report messages seem to end up in my junk folder. I currently don't use the pi at the moment so can't test the patch immediately (are you aiming for the 20.04 release?). The patch testing for the raspberry pi seems sensible. My only issue with your patch is dropping the "defaults". This seems to always be applied in pi images, but I've no clue if it is redundant.

> are you aiming for the 20.04 release

no; it's definitely too late for that, but possibly could happen as a SRU post-release.

> My only issue with your patch is dropping the "defaults". This seems to always be applied in pi images, but I've no clue if it is redundant.

Yeah, I was not sure if the options needed to be fully reset to 'defaults' or if just dropping the umask and/or gid was enough. It would help if we could understand why ubiquity is crashing with the umask/gid mount options.

If you're not able to test, I think it would make sense to let this bug sit until you, or anyone else, can resume testing/verification. As @waveform said, most people using Pi will be using pre-installed images.

I'm on CC for this bug, so whenever you (or, anyone else interested in this bug) is available to test (and then, verify) the patch, let me know.

The Eoan Ermine has reached end of life, so this bug will not be fixed for that release

I'm still on cc for this bug, so if someone can verify the proposed patch, I'll work on it, but I'm going to remove myself as assignee until that time.

[Expired for partman-basicfilesystems (Ubuntu Bionic) because there has been no activity for 60 days.]

[Expired for partman-basicfilesystems (Ubuntu Xenial) because there has been no activity for 60 days.]

[Expired for partman-basicfilesystems (Ubuntu) because there has been no activity for 60 days.]

[Expired for partman-basicfilesystems (Ubuntu Focal) because there has been no activity for 60 days.]