Created attachment 30622 [details] [review]
support creating minix file systems

Created attachment 30628 [details] [review]
support creating minix file systems

with proper "Bug %d -" prefix now.

(In reply to comment #1)
> Created an attachment (id=30622) [details]
> support creating minix file systems
> 

What I said in bug 24673 comment 8 probably applies here as well (it's worth testing this with Palimpsest). Thanks.

Created attachment 30631 [details] [review]
support creating minix file systems

Whoops, sorry. I updated the test suite to check the KnownFilesystems property (which catches that omission), reuploaded to people.c.c.

New patch attached.

(In reply to comment #4)
> Created an attachment (id=30631) [details]
> support creating minix file systems
> 
> Whoops, sorry. I updated the test suite to check the KnownFilesystems property
> (which catches that omission), reuploaded to people.c.c.
> 
> New patch attached.
> 

The patch is missing support for take_ownership_uid and take_ownership_gid options in the job-mkfs.c bits (by default, Palimpsest will check the "Take ownership of filesystem" checkbox if the fs claims to support unix owners.)

I'd add this myself, and was actually about to, but there's no /sbin/mkfs.minix available in Fedora ;-). So I'd kindly ask that the test-suite tries to omit testing minixfs since I wouldn't be able to run it then.

Or maybe it should check that we correctly return an error such as .MissingFilesystemTools with some detail specifying the fs identifier (e.g. 'minix') - real apps, like Palimpsest, could catch this error and invoke PackageKit to install the requisite software needed to support the filesystem. Should probably open a separate bug for this.

(In reply to comment #5)
> The patch is missing support for take_ownership_uid and take_ownership_gid
> options in the job-mkfs.c bits (by default, Palimpsest will check the "Take
> ownership of filesystem" checkbox if the fs claims to support unix owners.)

Oh, sorry for that. Will fix.
 
> I'd add this myself, and was actually about to, but there's no /sbin/mkfs.minix
> available in Fedora ;-). So I'd kindly ask that the test-suite tries to omit
> testing minixfs since I wouldn't be able to run it then.

It's built by util-linux-ng, so I assumed it'd be available anywhere. But no problem, easy enough to check whether mkfs.minix is there, and only run the tests then.

> Or maybe it should check that we correctly return an error such as
> .MissingFilesystemTools with some detail specifying the fs identifier (e.g.
> 'minix') - real apps, like Palimpsest, could catch this error and invoke
> PackageKit to install the requisite software needed to support the filesystem.

We could also fix the KnownFilesystems property to reflect that. Right now, can_create/can_mount are always true, even if e. g.  xfsprogs isn't installed or Fedora's u-l-ng doesn't build minix tools (where PackageKit wouldn't help either). Might be nicer than offering it in palimpsest and failing with an error.

Created attachment 30662 [details] [review]
support creating minix file systems

I updated the test suite to check take_ownership_*, which catches this omission.

Patch updated to support these options for minix, thanks for pointing out.

I also updated the test suite to skip the minix tests if mkfs.minix isn't available:

[...]
fs: minix ... [no mkfs, skip] ok
[...]

(In reply to comment #5)
> Or maybe it should check that we correctly return an error such as
> .MissingFilesystemTools with some detail specifying the fs identifier (e.g.
> 'minix') - real apps, like Palimpsest, could catch this error and invoke
> PackageKit to install the requisite software needed to support the filesystem.
> Should probably open a separate bug for this.

Bug 24718

Committed thanks. I did the gnome-disk-utility bits to use this in the new-ui branch, see

http://git.gnome.org/cgit/gnome-disk-utility/commit/?h=new-ui&id=86538c45143e32e5e42abd20807826266fa22084