Thanks for the MP Alexander.
Before we get too far into review, please understand that the decision to
include f2fs in the filesystems dropdown list of Ubuntu-desktop-installer or
Subiquity is not mine to make. Getting support for it in Curtin is just one
part. Look for example at
https://github.com/canonical/ubuntu-desktop-installer/issues/1332 - we removed
several filesystems from that list.
But if we do want to included f2fs at some point, it starts here, so let's
discuss this part.
docs: we should add an entry in the Format command - doc/topics/storage.rst
unittests: please consider the existing tests/unittests/test_block_mkfs.py
tests and think about a suitable f2fs version. tox -e py3-flake8 reports a
minor problem in curtin/block/schemas.py
integration tests: curtin has a body of vmtests, but the newer ones we tend to
run as part of tests/integration. I want to see more integration tests instead
of new vmtests. There isn't an example yet in integration on a new filesystem
type, so I won't make it required to add one there, but it'd be nice.
Diff comments:
> diff --git a/curtin/block/schemas.py b/curtin/block/schemas.py
> index 3278331..60807d9 100644
> --- a/curtin/block/schemas.py
> +++ b/curtin/block/schemas.py
> @@ -4,7 +4,7 @@ _uuid_pattern = (
> r'[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}')
> _path_dev = r'^/dev/[^/]+(/[^/]+)*$'
> _path_nondev = r'(^/$|^(/[^/]+)+$)'
> -_fstypes = ['btrfs', 'ext2', 'ext3', 'ext4', 'fat', 'fat12', 'fat16',
> 'fat32',
> +_fstypes = ['btrfs', 'ext2', 'ext3', 'ext4', 'f2fs', 'fat', 'fat12',
> 'fat16', 'fat32',
Linter will complain the line is too long, let's move an entry or two down a
line to keep it happy.
> 'iso9660', 'vfat', 'jfs', 'ntfs', 'reiserfs', 'swap', 'xfs',
> 'zfsroot']
> _ptable_unsupported = 'unsupported'
> diff --git a/debian/control b/debian/control
> index a35cbf6..0527580 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -25,6 +25,7 @@ Priority: extra
> Depends: bcache-tools,
> btrfs-progs | btrfs-tools,
> dosfstools,
> + f2fs-tools,
It's not desired to list every filesystem tools package here, curtin installs
missing packages needed on demand. It's better at this point to not list it
here, and we can ensure that f2fs-tools is in the right location in the cases
that want it.
> file,
> gdisk,
> lvm2,
--
https://code.launchpad.net/~nexusprism/curtin/+git/curtin/+merge/439880
Your team curtin developers is requested to review the proposed merge of
~nexusprism/curtin:master into curtin:master.
--
Mailing list: https://launchpad.net/~curtin-dev
Post to : [email protected]
Unsubscribe : https://launchpad.net/~curtin-dev
More help : https://help.launchpad.net/ListHelp
