GNOME Bugzilla – Bug 704893
Would be nice of have a function calculating the disk space of a directory
Last modified: 2018-05-24 15:33:38 UTC
Getting that information is something non trivial, that several applications need to do, it would be nice to have a shared implementation.
We could make this an attribute on GFileInfo... Doing it for local files is easy enough... for most other backends, we'd need to emulate it, and I can imagine it would be painfully slow, so we'd probably want async and cancellable. I imagine nautilus wouldn't use it for this reason because it shows progress as it's calculating folder sizes and GFileInfo doesn't really work that way...
For me, the only other backend that I care about at all is sftp... and it's interesting to note that sftp has a 'df' command, but seems to lack a 'du'.
Actually, looking at this a bit closer, I'm not sure this would be a good fit for GFileInfo.... the backends appear to assume that file info lookups are relatively cheap operations.... glocalfileinfo.c in GIO has no concept of async, even... Trying to mix a "simple" info lookup (like size, permission, etc) with a potentially long-running operation like this would prove to be difficult. I think we probably want to make this a completely separate call.
Created attachment 250324 [details] [review] GFile: add new g_file_query_disk_usage() API This is essentially the equivalent of 'du'. This is currently only supported on local files. gvfs will add support for the interface later.
Created attachment 250325 [details] [review] tests/: add gio-du This is basically a minimally-featured 'du' equivalent to manually test g_file_query_disk_usage().
Note: tested on Linux and under wine. Everything seems good except, under wine, opendir() succeeds even on permission errors, so we have no real way of detecting and reporting problems.
Created attachment 250337 [details] [review] Support GFile.query_disk_usage() Implement query_disk_usage() (and async variants) in GDaemonFile, adding support to the interface and to the daemon with a new job type. Add support for querying disk usage in the sftp backend by reusing the connection to execute 'du' remotely and parsing the results.
Created attachment 250338 [details] [review] tests/: add gio-du New version with some improvements, including --async mode.
This is fun :) [desrt@moonpix glib]$ gio/tests/gio-du --async -h -h sftp://velocity.lan/etc ~/Documents /home/desrt/Documents: 127.2 MB (127,229,952 bytes) sftp://velocity.lan/etc: 8.6 MB (8,630,272 bytes)
In retrospect, I'm thinking I should have called it "measure disk usage". Any opinions before I go on a mass renaming spree?
Review of attachment 250324 [details] [review]: Looks reasonable, though I'm not the best person to review this
Review of attachment 250337 [details] [review]: ::: daemon/gvfsjobquerydiskusage.h @@ +19,3 @@ + * + * Author: Alexander Larsson <alexl@redhat.com> + */ I guess this should be (c) Canonical and Author: desrt ?
Review of attachment 250338 [details] [review]: I think this would be better of as gvfs-du in gvfs/tools, where all the other commandline utilities live. ::: gio/tests/gio-du.c @@ +1,1 @@ +#include <gio/gio.h> Missing license header
Small discussion sniplet from IRC: 06:18 < alex> otherwise that sounds fine 06:19 < alex> although we tend to use goffset for file sizes 06:20 < alex> Thats signed though, so you loose some fidelity 06:20 < desrt> so i did it because of this: 06:20 < desrt> #define G_FILE_ATTRIBUTE_STANDARD_SIZE "standard::size" /* uint64 */ 06:22 < alex> desrt: yeah, no goffset in dbus. We have goffset in 06:22 < pbor> desrt: so I will be able to drop baobab's core :) 06:22 < alex> goffset g_file_info_get_size (GFileInfo *info); 06:22 < alex> though 06:22 < desrt> alex: hmm. 06:23 < desrt> so you would propose -1 is returned for errors? 06:23 < alex> Dunno about that 06:23 < desrt> 2**63 is on the order of 1000s of petabytes 06:24 < desrt> "should be enough for anybody" 06:24 < desrt> for me, the only benefit of using a signed type would be that i could return -1 on error 06:25 < desrt> and then skip the out parameter 06:25 < desrt> which would make it marginally nicer to use 06:25 < alex> yeah 06:25 < alex> i'd say go with that 06:25 < desrt> okay. i'll change it to goffset return value then 06:25 < desrt> everything else is OK? 06:25 < alex> yeah
06:31 < desrt> alex: you know... in changing this patch, i start to wonder about the return value stuff 06:31 < desrt> if (thing (&outval)) { ... } 06:31 < desrt> vs 06:31 < desrt> outval = thing () 06:31 < desrt> if (outval != -1) { ... } 06:32 < desrt> doubly so since -1 is a "magic value" in a way... 06:32 < alex> desrt: true. Although it binds better 06:32 < desrt> that's true. 06:32 < desrt> but i consider that a failing of the bindings layer 06:33 < alex> Yeah, every language should be C 06:33 < alex> :*) 06:33 < desrt> it's very obvious that gerror-with-boolean-return and out-param should be bound not as a function that returns a bool... 06:33 < alex> It can't be bound as a -1 06:33 < desrt> at least if you are binding to something with exceptions 06:33 < alex> but it can be done with exceptions if your language supports it 06:34 < desrt> our 'big 3' all throw gerror as exceptions 06:34 < alex> Anyway, if you feel strongly about it i think both approaches are fine 06:34 < desrt> i'll think about it more over lunch 06:34 < alex> its not like this is a function called in many places 06:35 < desrt> maybe will talk to colin as well about transforming functions that return gboolean-with-error and a separate outparam to being bound as returning that outparam 'natively' I talked to Colin and he envisions a future in which we could allow applications to opt-in to new bindings conventions (along the lines of a gi_set_version(2) call from main()) at which point we would fix the bindings of all of the functions like this one to have a 'proper' return value. As such, I'm going to make this API nice to use from C, leaving the bindings question to be solved later. As you note, it's not going to be a super-frequently-called function...
Created attachment 251067 [details] [review] GFile: add new g_file_measure_disk_usage() API Just a rename of the previous patch since I changed my mind about the only other issue....
About the gio-du patch: we're going to save this for a future 'gfile' commandline tool which will essentially replace the gvfs-* utilities (which don't actually have anything to do with gvfs in particular).
Review of attachment 251067 [details] [review]: ::: gio/gfile.h @@ +1106,3 @@ + GFileDiskUsageFlags flags, + GCancellable *cancellable, + guint64 *disk_usage, Nautilus also counts the nr of files and the nr of directories, i think we want to add that. Also, nautilus needs to do incremental counting, so would need some kind of progress callback... ::: gio/glocalfile.c @@ +2552,3 @@ + return FALSE; + +#ifdef AT_FDCWD I don't think you can hard-rely on the existance of openat & co meaning it works. It can be a symbol in libc, but the syscall returns ENOSYS due to no kernel support. @@ +2566,3 @@ + node = name; + filename = g_string_new ("file://"); + g_string_append (filename, node->data); This is *not* how you construct a url... You want g_uri_escape_string with allow_utf8==TRUE. @@ +2631,3 @@ + } + + if (disk_usage) Why would you ever pass NULL for the only out argument?
(In reply to comment #19) > +#ifdef AT_FDCWD > > I don't think you can hard-rely on the existance of openat & co meaning it > works. It can be a symbol in libc, but the syscall returns ENOSYS due to no > kernel support. RHEL6 has openat() at least. Personally, I don't think we need to go farther back than that.
Created attachment 254287 [details] [review] GFile: add new g_file_measure_disk_usage() API This is essentially the equivalent of 'du'. This is currently only supported on local files. gvfs will add support for the interface later.
Created attachment 254288 [details] [review] tests/: add gio-du This is basically a minimally-featured 'du' equivalent to manually test g_file_query_disk_usage().
Review of attachment 254287 [details] [review]: Looks ok to me. The one thing i wonder about is the size of directories. stat() is a bit weird here, as it says directories are 0 blocks, but have a size... Dunno if this matters much, but maybe we should look at what du does.
Review of attachment 254288 [details] [review]: ::: gio/tests/gio-du.c @@ +77,3 @@ + gint i; + + setlocale (LC_ALL, ""); textlocale? bindtextlocale() etc?
Review of attachment 254287 [details] [review]: fwiw, I tested against 'du' and I get the same results for both block and apparent sizes, so I guess I'm already doing the same thing.
Review of attachment 254288 [details] [review]: ::: gio/tests/gio-du.c @@ +77,3 @@ + gint i; + + setlocale (LC_ALL, ""); Not likely to be the most useful things in the world, since this is just a test program, but sure...
Review of attachment 254288 [details] [review]: ::: gio/tests/gio-du.c @@ +77,3 @@ + gint i; + + setlocale (LC_ALL, ""); Actually, not even. This program contains no translated strings, so setting the default text domain or binding it is kinda pointless.
Attachment 254287 [details] pushed as 6ec2bb1 - GFile: add new g_file_measure_disk_usage() API Attachment 254288 [details] pushed as a61c9f4 - tests/: add gio-du Leaving open for the gvfs support, which will require somewhat more work now because of the progress reporting...
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/736.