After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 525482 - GIO/GVFS Port
GIO/GVFS Port
Status: RESOLVED FIXED
Product: gthumb
Classification: Other
Component: general
2.10.x
Other Linux
: Normal enhancement
: ---
Assigned To: Paolo Bacchilega
Paolo Bacchilega
: 560696 (view as bug list)
Depends on: 584920 584931 588125
Blocks: 533133 583464 588322
 
 
Reported: 2008-04-01 06:04 UTC by Gabriel Falcão
Modified: 2009-09-10 17:51 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Current port state (7.42 KB, patch)
2008-04-01 06:08 UTC, Gabriel Falcão
none Details | Review
Patch update. src/rotation-utils.c ported to GIO (17.58 KB, patch)
2008-04-02 04:58 UTC, Gabriel Falcão
none Details | Review
Fixing (17.34 KB, patch)
2008-04-02 22:20 UTC, Gabriel Falcão
none Details | Review
GFileInfo and GAppInfo related ports (24.11 KB, patch)
2008-05-17 05:16 UTC, Gabriel Falcão
committed Details | Review
sub-patch for open-width functions (7.61 KB, patch)
2008-05-19 17:28 UTC, Michael Chudobiak
committed Details | Review
More ports (22.94 KB, patch)
2008-08-06 15:04 UTC, Gabriel Falcão
committed Details | Review
GIO port -- use GFile in web exporter (31.46 KB, patch)
2008-08-07 22:05 UTC, Christophe Bisière
committed Details | Review
Fix png exporter (creation of the image map html file) (1.06 KB, patch)
2008-08-09 08:50 UTC, Christophe Bisière
committed Details | Review
Example gio & wrapper: path_is_dir, path_is_file (9.86 KB, patch)
2008-08-09 16:48 UTC, Christophe Bisière
none Details | Review
gio & wrapper (path_is_*, ensure_dir_exists) (13.77 KB, patch)
2008-08-09 20:20 UTC, Christophe Bisière
none Details | Review
gio wrapper, last version (21.56 KB, patch)
2008-08-10 07:55 UTC, Christophe Bisière
none Details | Review
gio wrapper, last version (26.75 KB, patch)
2008-08-10 09:13 UTC, Christophe Bisière
committed Details | Review
More GIO work on gfile-utils.c & catalog-web.exporter.c (20.74 KB, patch)
2008-08-11 10:11 UTC, Christophe Bisière
none Details | Review
More GIO work on gfile-utils.c & catalog-web.exporter.c (v2) (30.24 KB, patch)
2008-08-11 12:42 UTC, Christophe Bisière
needs-work Details | Review
More GIO work on gfile-utils.c & catalog-web.exporter.c (against trunk) (14.16 KB, patch)
2008-08-11 13:08 UTC, Christophe Bisière
committed Details | Review
Replace gnome_vfs_xfer_uri with file_copy (2.95 KB, patch)
2008-08-11 17:53 UTC, Christophe Bisière
committed Details | Review
implemented the async_xfer_file_gio, to be used as replacement for gnome_vfs_async_xfer... stuff (10.30 KB, patch)
2008-08-12 21:38 UTC, Gabriel Falcão
needs-work Details | Review
Improved async copy and move (5.20 KB, patch)
2008-08-13 10:42 UTC, Gabriel Falcão
needs-work Details | Review
Improved async copy and move [fixed] (13.03 KB, patch)
2008-08-13 19:08 UTC, Gabriel Falcão
none Details | Review
Improved async copy and move (fixed indentation) (13.11 KB, patch)
2008-08-14 04:45 UTC, Gabriel Falcão
none Details | Review
Improved async copy and move (fixed indentation) (181.71 KB, patch)
2008-08-14 14:21 UTC, Gabriel Falcão
needs-work Details | Review
GIO port -- mime (12.80 KB, patch)
2008-08-16 22:27 UTC, Christophe Bisière
none Details | Review
convert MIME and use output stream in web exporter (35.00 KB, patch)
2008-08-17 21:13 UTC, Christophe Bisière
none Details | Review
Sending files to trash (195.96 KB, patch)
2008-08-19 06:00 UTC, Gabriel Falcão
needs-work Details | Review
MIME & stream (39.54 KB, patch)
2008-08-19 08:43 UTC, Christophe Bisière
committed Details | Review
trash stuff done (181.36 KB, patch)
2008-08-30 19:19 UTC, Gabriel Falcão
none Details | Review
Fix g_propogate_warnings assertion when there is no error. (430 bytes, patch)
2008-08-31 20:57 UTC, dynamotwain
committed Details | Review
Trash stuff done and stripped (45.99 KB, patch)
2008-09-05 03:49 UTC, Gabriel Falcão
none Details | Review
Trash stuff done, stripped and checked (46.66 KB, patch)
2008-09-11 02:19 UTC, Gabriel Falcão
needs-work Details | Review
isolate vfs code in dlg-file-utils.c (18.64 KB, patch)
2009-01-03 18:36 UTC, Christophe Bisière
committed Details | Review
GIO conversion of path_list_async_new and associated functions. (9.37 KB, patch)
2009-06-02 02:03 UTC, Marlodavampire
committed Details | Review
GIO root path (Filesystem) string updates (2.70 KB, patch)
2009-06-02 02:06 UTC, Marlodavampire
committed Details | Review
GIO Fix .. element in location sidebar (1.34 KB, patch)
2009-06-02 15:43 UTC, Marlodavampire
needs-work Details | Review
GIO Fix .. element in location sidebar (1.35 KB, patch)
2009-06-03 03:38 UTC, Marlodavampire
committed Details | Review
GIO src/gth-location.c converstion (3.68 KB, patch)
2009-06-03 16:32 UTC, Marlodavampire
rejected Details | Review
GIO src/gth-location.c converstion (12.43 KB, patch)
2009-06-03 16:45 UTC, Marlodavampire
committed Details | Review
GIO Fix error passing on directory load (4.28 KB, patch)
2009-06-04 04:11 UTC, Marlodavampire
none Details | Review
GIO Fix error passing on directory load (4.70 KB, patch)
2009-06-04 04:31 UTC, Marlodavampire
needs-work Details | Review
GIO Fixes to allow remote mount browsing (4.73 KB, patch)
2009-06-04 12:03 UTC, Marlodavampire
committed Details | Review
GIO Fix error passing on directory load (4.83 KB, patch)
2009-06-04 13:13 UTC, Marlodavampire
committed Details | Review
GIO Fix bookmarks to unmounted locations (1.57 KB, patch)
2009-06-04 15:32 UTC, Marlodavampire
committed Details | Review
GIO use ITEMS_PER_NOTIFACTION for g_file_enumerator_next_files_async (794 bytes, patch)
2009-06-17 00:47 UTC, Marlodavampire
none Details | Review
GIO conversion of dlg-duplicates to GFile (23.08 KB, patch)
2009-06-18 06:09 UTC, Marlodavampire
committed Details | Review
GIO dlg-search conversion (15.54 KB, patch)
2009-06-22 06:18 UTC, Marlodavampire
committed Details | Review
GIO Error handling (5.06 KB, patch)
2009-06-25 03:23 UTC, Marlodavampire
committed Details | Review
GIO port dlg-file-utils delete and trash functions to GFile (27.87 KB, patch)
2009-07-03 12:32 UTC, Marlodavampire
committed Details | Review
GIO port dlg-file-utils to GFile (56.83 KB, patch)
2009-07-15 22:40 UTC, Marlodavampire
none Details | Review

Description Gabriel Falcão 2008-04-01 06:04:04 UTC
According to Gnome defaults, all Gnome applications must be ported to GIO/GVFS.

I am currently working on it.

Here are some entries of my changed ChangeLog :)

* src/main.c: Removed GnomeVFS dependencies. Just little changes like
g_uri_escape_string in place of gnome_vfs_escape_string.

* libgthumb/file-utils.c: in get_file_size, using GFileInfo instead of
gnome_vfs_get_file_info.

* src/dlg-image-prop.c, src/gth-fullscreen.c: Using g_format_size_for_display instead of gnome_vfs_format_file_size_for_display, according to libgthumb/file-utils.c get_file_size, new implementation, that returns a goffset.

I read almost all gthumb code, and i think that i am about to take 2 weeks to finish all port.
Comment 1 Gabriel Falcão 2008-04-01 06:08:15 UTC
Created attachment 108393 [details] [review]
Current port state

This is just the current patch, i have no hope to this be useful, anyway here is the patch.
Comment 2 Gabriel Falcão 2008-04-02 04:58:45 UTC
Created attachment 108452 [details] [review]
Patch update. src/rotation-utils.c ported to GIO

Updating the patch, while this report is not confirmed, i will post the patches here, to keep updated.
New ChangeLog entry:
2008-04-02  Gabriel Falcão <gabriel@nacaolivre.org>

	* src/rotation-utils.c: g_file_query_info and
	g_file_set_attributes_from_info used,
	instead of gnome_vfs_get_file_info and gnome_vfs_set_file_info.
	It removes the GnomeVFS dependency from thid file.
Comment 3 Michael Chudobiak 2008-04-02 12:37:57 UTC
Hi Gabriel, 

gThumb definitely needs to be ported to gio/gvfs, so thanks for taking a look at it!

I'm still learning gio/gvfs myself, so I'm not am expert on it. However, here are a few comments based on reading on your most recent patch:

1) Something is wrong with your tabs/spacing. You've only added one new line to configure.in, but look how many +/- lines are in the diff file. Please tidy that up. 

2) You might as well add a "$GIO_REQUIRED" variable, for consistency with the rest of configure.in.

3) Remove your "g_print" debugging lines.

4) Follow the existing code style. Use

if (foo) {
        bar;
}

not

if (foo)
{
  bar;
}

That means 8-space or 1-tab indent, not two spaces. Use the bracketing style shown above. This is very important for readability. You'll need to change your editor's settings.

5) I don't think we need GCancellable *cancel. We don't use the cancel feature, so you can just supply NULL to g_file_query_info.


- Mike
Comment 4 Gabriel Falcão 2008-04-02 22:20:11 UTC
Created attachment 108508 [details] [review]
Fixing

Hi Michael,
i was fixed the typos that you said.
Smething really weird was happened to my vim (or with me xD), but now i will follow the gthumb code guidelines by a very strict way.
Should i keep putting all the modifications that i made as a patch here on bugzilla or sending to another place ?

Thanks about your patience and time, i am very excited and proud to contribute with gthumb.
Comment 5 Michael Chudobiak 2008-04-03 12:31:19 UTC
Some more comments:

1) Your Changelog entries are not up to date - they do not include all affected files.

2) Formatting issue:

+        GFile *file_gfile;
+        GFileInfo *info;
+        GError *newerror = NULL;

should be aligned like

+        GFile     *file_gfile;
+        GFileInfo *info;
+        GError    *newerror = NULL;

3) I would shorten "file_gfile" to just "gfile", personally.

4) I wonder if the GFile should be made a part of the FileData struct, so that we only call g_file_new_for_uri from file_data_new. What do you think?

- Mike
Comment 6 Michael Chudobiak 2008-04-03 12:32:13 UTC
Also: Yes, all patches should be attached to this bug report.

- Mike
Comment 7 Gabriel Falcão 2008-04-04 14:51:38 UTC
Hi Michael,

thanks for the new issues, i will take a look at it.
About the file_gfile, i agree to it.

I am currently working on some parts of libgthumb (file-utils and file-data, to be more specific), and i've added GFile as part of PathListData.
I haven't saw all use of FileData, but i think that a GFile will be welcome.
Because i'm working on something that needs a lot of care, my next patch maybe take a little late.
In next patch i hope that i can fill all issues about formating, code style and so.

Thanks a lot!
Comment 8 Matthias Clasen 2008-05-05 04:01:30 UTC
One thing that should be ported as a priority is the trash handling, since that causes an actual regression. Nautilus now uses the trash spec location (~/.local/share/Trash, whereas gthumb still uses the pre-trash-spec gnome-vfs location 
of ~/.Trash). This means that images that are trashed in gthumb don't show up in the trash window of nautilus.

Thankfully, gio has a g_file_trash() function that should make this pretty easy.

Downstream bug about this issue: 
https://bugzilla.redhat.com/show_bug.cgi?id=445125
Comment 9 Michael Chudobiak 2008-05-10 17:11:31 UTC
Gabriel,

I've committed part of your patch, mostly the gnome_vfs_format_file_size_for_display -> g_format_size_for_display stuff.

Numerous variables also had to be changed from GnomeVFSFileSize -> goffset.

Are you doing further porting? If so, please follow the code used in file-roller, which is Paolo's other big project. It has already been ported.

- Mike
Comment 10 Gabriel Falcão 2008-05-17 05:16:57 UTC
Created attachment 111031 [details] [review]
GFileInfo and GAppInfo related ports

Mike,
as you asked me by mail, here follows the port of src/dlg-image-prop.c, src/dlg-open-with.c and resolving the problem at src/gth-viewer.c that caused a reversion in revision [2331].

Please check the ChangeLog for more details.

Regards,
Gabriel Falcão
Comment 11 Michael Chudobiak 2008-05-18 17:52:51 UTC
The dlg-image-prop.c stuff has been committed and copied to gth-exif-data-viewer.c as well.

I'm still reviewing the other bits.

- Mike
Comment 12 Michael Chudobiak 2008-05-19 17:28:31 UTC
Created attachment 111170 [details] [review]
sub-patch for open-width functions

OK, focusing on the open-with parts of the patch next (stripped-down patch attached), I get these warnings:

(gthumb:18372): GLib-GObject-WARNING **: IA__g_object_new_valist: object class `GThemedIcon' has no property named `names'

(gthumb:18372): GLib-GIO-CRITICAL **: g_themed_icon_constructed: assertion `themed->names != NULL && themed->names[0] != NULL' failed

Can that be fixed, Gabriel?

- Mike
Comment 13 Gabriel Falcão 2008-05-20 17:29:48 UTC
Hi Mike,

it seems to be a problem with GIO, i am using the version 2.16.3 packaged in Debian sid, but checking the svn log of Glib/GIO i saw a fix for that problem:

http://svn.gnome.org/viewvc/glib?view=revision&revision=6666

i did test the gio at revision 6666 with mine last gthumb patch and it works great. Maybe i am doing something wrong :(

 - Gabriel Falcão
Comment 14 Michael Chudobiak 2008-05-22 00:00:46 UTC
OK, the "open with" stuff has been committed:

http://svn.gnome.org/viewvc/gthumb?rev=2335&view=rev

I'll check the owner/permission copying bits next. Why did you include size in those gfileinfo calls?

- Mike
Comment 15 Morten Welinder 2008-06-10 13:34:29 UTC
> (gthumb:18372): GLib-GObject-WARNING **: IA__g_object_new_valist: object class
> `GThemedIcon' has no property named `names'

That's bug 537555.
Comment 16 André Klapper 2008-07-19 11:02:57 UTC
any news here?
Comment 17 Michael Chudobiak 2008-07-21 12:42:36 UTC
Hmm, I've forgotten to look at the g_file_query_info patch bits. I'll look at them and get them committed.

However, the hard parts of the gio migration (the file xfer stuff) are basically stalled... Gabriel, are you looking at it? If not, Paolo will need to when he has free time. It's beyond my skill level.


- Mike
Comment 18 Gabriel Falcão 2008-07-22 23:35:01 UTC
This month i am looking at other projects, actually i need to close a bunch of unterminated projects, but this Gthumb port is in my list, and in the first week of August i will be attending to debian camp, and the Gthumb GIO port have the highest priority.

The xfer stuff is beyond my skill level too, but have "almost simple" in which i want to work on.
Comment 19 Gabriel Falcão 2008-08-06 15:04:31 UTC
Created attachment 115980 [details] [review]
More ports

Porting the file_rename and xfer_file functions (libgthumb/file-utils.[ch]) to GIO, and porting all the stuff which use the file_rename to the new function signature.
Migrating all CopyDoneFunc
Comment 20 Michael Chudobiak 2008-08-06 19:21:58 UTC
Gabriel,

Great to see progress!

Could you merge the non-committed bits of patch 111031 with the full 115980 patch, so that I can apply & test cleanly against svn rev 2381 (the latest)?

Some parts of patch 111031 were not committed last time, because I didn't see the point of loading "standard::size" in the g_file_info calls. Please remove that.

Thanks!

- Mike
Comment 21 Christophe Bisière 2008-08-06 22:32:16 UTC
Gabriel, Mike,

That is good news, indeed!

On my side, I started to GFile-ify catalog-web-exporter.[ch].

Replacing paths and uris with GFile wherever it makes sense greatly simplifies the code.

With the great work done on xfer functions by Gabriel, the Web exporter will be soon very gio friendly!

I am almost done. I will post a patch when all the pending patches that Mike refers to are committed.

Note that I had to import a few functions from file-roller:file-utils.c.

Bests,

Christophe
Comment 22 Michael Chudobiak 2008-08-07 14:16:37 UTC
OK, I merged the pending patches and committed them with minor corrections. Thanks Gabriel!

The Trash handling still needs to be fixed.

- Mike
Comment 23 Christophe Bisière 2008-08-07 22:05:12 UTC
Created attachment 116102 [details] [review]
GIO port -- use GFile in web exporter

Mike, Gabriel,

Here is a first step toward the GFile-ification of the web exporter. This patch is limited to catalog-web-exporter.[ch].

In the context of the GIO/GVFS port, I think it would be great to use GFile as our preferred data structure for representing file paths/uris, converting to char* only when it is necessary (e.g. for display). This will certainly simplify the code.

The transition might be quite difficult, though, since char* representations for file paths/uris are spread everywhere in the code. A possible strategy could be to implement (or borrow form other projects like file-roller) GFile versions of the main files/directories functions (starting from file-utils.c), plus a wrapper matching the current api (that is, using char* as parameters). That way, it would be possible to migrate progressively to the GFile-api, and get rid of the char* wrappers at the end of the process.

What do you think?

Best Regards,

Christophe
Comment 24 Michael Chudobiak 2008-08-08 12:01:05 UTC
Christophe,

Yes, that's sort of what I was suggesting in comment 5 - put the gfile in the FileData struct, and start using it as the main file identifier, rather than path/uri. For a while we would need both the gfile and path/uri in FileData, but later the path/uri would be removed.

Hopefully that would eliminate a lot of the annoying escaping / encoding / charset issues that constantly pop up.

I don't think it would be that hard to do, but it would take a while.

So if you feel ambitious, please go ahead!

I'll check your patch later today, hopefully.

- Mike
Comment 25 Gabriel Falcão 2008-08-08 15:51:22 UTC
Working on file copy_file_async (done!), dlg_files_move_to_trash (done!) and dlg_folder_move_to_trash (working on).
I am making some tests with g_io_scheduler_push_job, and i'm having some sucessfulness.
Trash stuff almost done.

Where are these stuff:
libgthumb/file-utils.[ch]
src/dlg-file-utils.[ch]

Comment 26 Michael Chudobiak 2008-08-08 16:01:48 UTC
(In reply to comment #25)
> Working on file copy_file_async (done!), dlg_files_move_to_trash (done!) and
> dlg_folder_move_to_trash (working on).
> I am making some tests with g_io_scheduler_push_job, and i'm having some
> sucessfulness.
> Trash stuff almost done.

Yay!


> Where are these stuff:
> libgthumb/file-utils.[ch]
> src/dlg-file-utils.[ch]

Huh? What do you mean?

- Mike


Comment 27 Michael Chudobiak 2008-08-08 16:58:34 UTC
Christophe,

Why do you have this in your patch...

static char *
file_get_uri (GFile *file)
{
        return g_file_get_uri (file);
}

Shouldn't you just call g_file_get_uri directly?

- Mike
Comment 28 Gabriel Falcão 2008-08-08 19:48:23 UTC
(In reply to comment #26)
> > Where are these stuff:
> > libgthumb/file-utils.[ch]
> > src/dlg-file-utils.[ch]
> 
> Huh? What do you mean?
> 
> - Mike
> 
Sorry, it is because my poor english :(
I was tryng to say that the functions which i am working on, are located in these files.

I keep working in the trash stuff, my tests are almost done, i am just dealing with asyncronous I/O stuff...

Comment 29 Christophe Bisière 2008-08-08 22:13:12 UTC
(In reply to comment #27)
> Christophe,
> 
> Why do you have this in your patch...
> 
> static char *
> file_get_uri (GFile *file)
> {
>         return g_file_get_uri (file);
> }
> 
> Shouldn't you just call g_file_get_uri directly?
> 
> - Mike
> 

Mike, 

I wrote a set of functions in catalog-web-exporter, organized in two layers: 

1) functions returning an object whose type is our new "internal" representation of a file name (that is, a GFile)

2) functions converting from this internal representation to various (char*-based) "external" formats (for display or other purposes). These formats are paths and uris, relative or absolute.

I think it is better for general readability and maintenability that a piece of code that needs to perform one of these "generic" tasks (i.e. creating an instance of our internal representation, or tranforming into an external representation) explicitly calls a function in the proper layer. It gives use better control over the callers (who knows, maybe one day we will figure out that we have much more to do into file_get_uri than just a g_file_get_uri), and it facilitates debugging.

So this "file_get_uri" is just a side effect of my programming style: I do like layers that callers are "forced" to hook into.

But of course feel free to edit!

Bests,

Christophe
Comment 30 Christophe Bisière 2008-08-09 08:50:54 UTC
Created attachment 116212 [details] [review]
Fix png exporter (creation of the image map html file)

rev 2382 broke the png exporter. This is a quick fix.

Christophe
Comment 31 Christophe Bisière 2008-08-09 09:18:03 UTC
(In reply to comment #29)

> 2) functions converting from this internal representation to various
> (char*-based) "external" formats (for display or other purposes). These formats are paths and uris, relative or absolute.

Note that these four functions may move to file-utils.c and become mandatory for GFile->char* conversions.

The UNREF macro may be moved too.

Another possibility is to create a new gfile-utils.[ch] where we would progressively move gio-ported file-utils.[ch] functions, leaving only path/uri wrappers into file-utils.[ch].

Getting rid of file-utils.[ch] would be one goal of the gio port.

Christophe
Comment 32 Michael Chudobiak 2008-08-09 10:20:36 UTC
Christophe,

OK, I committed your two patches (rev 2388).

Creating gfile-utils.[ch] is probably a good idea. Gabriel, can you move your updated functions from file-utils.c into gfile-utils.c?

I have created empty gfile-utils.[ch] files (rev 2389).

- Mike
Comment 33 Christophe Bisière 2008-08-09 11:46:55 UTC
Thanks Mike.

May I kindly suggest that we give a special prefix to public functions in gfile-utils.[ch] such that it becomes obvious if a caller has been ported or not?

For instance, for a function 

  foo (char *)

in file-utils, we could use 

  gfile_foo (GFile *) 

in gfile-utils and keep 

  foo (char *) 

as a small wrapper in file-utils.

At the end of the process, we may get rid of the gfile_ prefixes, or keep them. But in the meantime it could prove useful.

My 2¢!

Christophe
Comment 34 Michael Chudobiak 2008-08-09 12:42:09 UTC
Sure, gfile_foo sounds good.

It would of course be nice to completely replace foo with gfile_foo where possible, rather than keeping wrapper functions around... but I suppose things could be messy for a while :-)

- Mike
Comment 35 Christophe Bisière 2008-08-09 16:48:49 UTC
Created attachment 116239 [details] [review]
Example gio & wrapper: path_is_dir, path_is_file

Mike,

Here is an example of this "gio + wrapper" approach, applied to path_is_dir & path_is_file. The gio part is based on a function found in file-roller. I just disentangled the GFile* from the char* interface.

Note that this patch did not convert any caller. I think we should first make sure that a wrapping does not introduce new bugs by itself before converting the callers.

Bests,

Christophe
Comment 36 Christophe Bisière 2008-08-09 20:20:28 UTC
Created attachment 116262 [details] [review]
gio & wrapper (path_is_*, ensure_dir_exists)

More: ensure_dir_exists (with caller catalog-web-exporter.c converted).

Chr.
Comment 37 Gabriel Falcão 2008-08-10 07:03:54 UTC
Mike, Chris,
i still working on trash stuff.
Actually, i spent lot of time writting stuff to test and learn how to use g_io_scheduler stuff.
Since Gthumb are very stuck in the rough way that GnomeVFS does async stuff, i decided to write a functions for the libgthumb/file-utils that have the same (or almost at least), behavior and interface than gnome_vfs_async_*.
For now, i wrote the async_xfer_file_gio, and am migrating stuff like xfer_file and so on to use it.

Nevertheless, i was using GIT to control my modifications, but since the last time i tried to make a git pull (unsuccessfully) i had to go back to svn, and it is making me do the things without the organization i wanted to.

But as i said, i still working on it, and hopefully today i will send a patch with lots of modifications.

PS.: I am having so much dificulties to deal with the gthumb name conventions, because sometimes a function have a parameter like char *source_uri, but actually it receives a path, and sometimes the inverse happens, so i am using the g_file_new_for_commandline_arg to create gfiles instead of specific g_file_new_for_uri or g_file_new_for_path, since g_file_new_for_commandline_arg recognizes the which kind of parameters is dealing with.
Comment 38 Christophe Bisière 2008-08-10 07:21:31 UTC
(In reply to comment #37)

Gabriel,

On my side, I am still working on the gfile/char* split (see comment #33).

path_is_file, path_is_dir, and ensure_dir_exists are on my pending patch 116262.

I am currently converting get_destination_free_space () and get_temp_dir_name ().

To avoid the path/uri mess we had in file-util.c, I am enforcing the following policy: all the functions in gfile-util.c only accept/return uri-style GFiles.

Your get_file_size () is a good candidate for a split. Do you want me to do it as part of the next version of my patch? (the interface in file-util.c will not change, so it should be transparent for you).

Bests,

Christophe
Comment 39 Christophe Bisière 2008-08-10 07:55:19 UTC
Created attachment 116277 [details] [review]
gio wrapper, last version

Adding:
  get_destination_free_space () 
  get_temp_dir_name ()

and eating my own dog food! (in catalog-web-exporter.c)

Note: I am enforcing the following policy: all the functions in gfile-util.c only accept/return uri-style GFiles.

Bests,

Christophe
Comment 40 Gabriel Falcão 2008-08-10 08:29:15 UTC
Thanks for explaining Chris :)
I would like if you split the get_file_size, i was testing he exactly now...
I am up to convert all the gnome_vfs_async_xfer, to the mine async_xfer_file_gio, but when moving a file from my desktop to gthumb window, i got a segfault :(
And it is something related to getting inexistent attributes of GFiles, anyway, ot should not happen. I think that my version of Glib/GIO is with bugs...

And oh... you are doing a great job! Congrats!
Comment 41 Christophe Bisière 2008-08-10 09:13:32 UTC
Created attachment 116278 [details] [review]
gio wrapper, last version

Adding:
  dir_remove_recursive ()
  get_file_size ()

Bests,

Christophe
Comment 42 Christophe Bisière 2008-08-10 09:22:20 UTC
(In reply to comment #40)
> Thanks for explaining Chris :)
> I would like if you split the get_file_size, i was testing he exactly now...
> I am up to convert all the gnome_vfs_async_xfer, to the mine
> async_xfer_file_gio, but when moving a file from my desktop to gthumb window, i
> got a segfault :(
> And it is something related to getting inexistent attributes of GFiles, anyway,
> ot should not happen. I think that my version of Glib/GIO is with bugs...
> 
> And oh... you are doing a great job! Congrats!
> 

Thank you Gabriel.

Compared to what you are dealing with, cleaning up file-utils / gfile-utils is piece of cake!

BTW, I converted your get_file_size ().

Bests,

Christophe
Comment 43 Michael Chudobiak 2008-08-10 11:58:25 UTC
Great stuff, guys!

The gfile code is so much cleaner than the old char*-based stuff.

The wrapper approach looks nice too.

Christophe's patch has been committed to rev 2391.

- Mike
Comment 44 André Klapper 2008-08-10 12:47:52 UTC
correcting patch status as per last comment.
Comment 45 Christophe Bisière 2008-08-11 10:11:07 UTC
Created attachment 116326 [details] [review]
More GIO work on gfile-utils.c & catalog-web.exporter.c

Mike, Gabriel,

This patch contains:
- bug fixes
- more work on the Web exporter
- more work on gfile-utils.c
  in particular: introduce two constructors enforcing the uri-only policy: 
  gfile_new (path) and gfile_new_va (paths1, path2, ..., NULL).
  Please use them!

Using these constructors simplifies a bit the xfer functions written by Gabriel in file-utils.c

Bests,

Christophe
Comment 46 Christophe Bisière 2008-08-11 12:42:04 UTC
Created attachment 116340 [details] [review]
More GIO work on gfile-utils.c & catalog-web.exporter.c (v2)

Better patch:

- kill more gnome_vfs stuffs in catalog-web-exporter.c
- split Gabriel's file_copy, file_move, file_rename 
  (side note: I dropped the third parameter of file-utils.c:file_name since nobody uses it. We could reintroduce it latter in a gfile-utils.c:file_name if needed)

The split is currently clean (but far form complete, of course): GFiles in file-utils.c are only used in wrappers.

Bests,

Christophe
Comment 47 Michael Chudobiak 2008-08-11 12:56:29 UTC
Christophe, I committed v1 of your patch just before v2 appeared. Can you re-submit v2 against current trunk? Thanks!

The new gfile-creation code is very elegant.

- Mike
Comment 48 Christophe Bisière 2008-08-11 13:08:08 UTC
Created attachment 116341 [details] [review]
More GIO work on gfile-utils.c & catalog-web.exporter.c (against trunk) 

Sure. --Chr.
Comment 49 Michael Chudobiak 2008-08-11 14:07:00 UTC
Christophe,

You probably didn't want a pointer here:

gboolean   *copy_done;

catalog-web-exporter.c: In function 'export__copy_image':
catalog-web-exporter.c:2664: warning: assignment makes pointer from integer without a cast

- Mike
Comment 50 Christophe Bisière 2008-08-11 14:09:33 UTC
Oops, indeed! --Chr.
Comment 51 Michael Chudobiak 2008-08-11 14:36:07 UTC
Committed to 2393, with above correction.

- Mike
Comment 52 Christophe Bisière 2008-08-11 17:53:04 UTC
Created attachment 116374 [details] [review]
Replace gnome_vfs_xfer_uri with file_copy

Low hanging fruit!

Bests,

Chr.
Comment 53 Michael Chudobiak 2008-08-11 18:21:00 UTC
Ooh, I like patches with many more "-" signs than "+" signs!

Committed, rev 2394.

- Mike
Comment 54 Gabriel Falcão 2008-08-12 21:38:09 UTC
Created attachment 116457 [details] [review]
implemented the async_xfer_file_gio, to be used as replacement for gnome_vfs_async_xfer... stuff

This function has been tested in a rough way, but works pretty.
I tried to migrate the xfer_file function from src/dlg-file-utils.c, but i got infinites segfaults, so i decided to send this patch and rewrite the xfer_file function.
Comment 55 Michael Chudobiak 2008-08-13 00:28:44 UTC
Gabriel,

After a quick review, I see one major problem: the trash implementation is too simple. user_data_dir/Trash is not the only possible trash location. There can be "top-level" trash dirs too.

We really need to use g_file_trash, even though it isn't a nice fit for the existing code. It will figure out the correct trash location for us.

Also, the new xfer code should probably go in gfile-utils.c, not file-utils.c. 

Christophe, can you comment as well?

- Mike
Comment 56 Christophe Bisière 2008-08-13 07:47:23 UTC
Hi Gabriel,

I agree with Mike. As stated in

  http://library.gnome.org/devel/gio/unstable/ch15.html

g_file_trash () is the way to go.

With respect to your xfer code, I can't wait using it! 

But before, I would be glad if you could sync it with the gfile-utils / file-utils split. 

For instance, your source_list, destination_list could be GList's of GFile's, in order to avoid using g_file_new_for_commandline_arg () in _async_xfer_job_do (). That way you will be able to move char*-free code to gfile-utils, and leave only a small wrapper into file-utils (use gfile_ as prefix for public functions).

You could use the already migrated code as reference examples.

Also, please uses the GFile constructors gfile_new () and gfile_new_va () when you need to create a GFile from a char* path.

Hey, we are getting closer!

Christophe
Comment 57 Gabriel Falcão 2008-08-13 10:42:43 UTC
Created attachment 116489 [details] [review]
Improved async copy and move

Chris, you have made a awesome work with that gfile-utils features!
That thing have just avoided me to get segfaults! :D

I hope this patch be useful :)
Mike, i haven't maked the trash stuff as well because my brain got insanely in lust to resolve the asyncronous I/O stuff, now i can have a nice work.
Today i will work on trash stuff.

Thanks a lot, guys, i am learning so much things !
Comment 58 Michael Chudobiak 2008-08-13 11:52:05 UTC
Gabriel,

Thanks for the work!

However, the patch seems to be malformed - it is missing async_xfer_file_gio from gfile-utils.c and other code. Please check it and re-submit.

- Mike
Comment 59 Gabriel Falcão 2008-08-13 19:08:22 UTC
Created attachment 116516 [details] [review]
Improved async copy and move [fixed]

Sorry Mike, now i am sending the full patch. My bad :(
Comment 60 Michael Chudobiak 2008-08-13 19:24:36 UTC
Thanks Gabriel!

Please fix the indentation/bracketing style to follow gthumb conventions (see comment 3 for a reminder).

- Mike
Comment 61 Gabriel Falcão 2008-08-14 04:45:48 UTC
Created attachment 116545 [details] [review]
Improved async copy and move (fixed indentation)

I think now the indentation is ok.
Comment 62 Michael Chudobiak 2008-08-14 11:49:15 UTC
It still has non-standard waste-spacing bracketing. Change

if (foo == bar)
        {
                do_func (bla);
        }

to

if (foo == bar) {
        do_func (bla);
}

You can use:

astyle --style=linux < infile > outfile

to fix style errors.
Comment 63 Michael Chudobiak 2008-08-14 11:51:29 UTC
Gabriel & Christophe,

I will be away for ~5 days.

Christophe, can you review/test/modify Gabriel's patches and merge them into your patches, if you are working on gthumb over the next week?

- Mike
Comment 64 Gabriel Falcão 2008-08-14 14:21:28 UTC
Created attachment 116576 [details] [review]
Improved async copy and move (fixed indentation)

I did use the astyle.
Fortunely, comparing the new diff i found a little memory leak in my patch, so i just had fixed it.
Comment 65 Christophe Bisière 2008-08-14 15:52:30 UTC
Hello Gabriel,

Your patch is almost 5000 lines, mainly due to tab/8 spaces conversions on existing code. I am not sure it is a good thing to mix in a single patch formatting and new real content. In particular, the svn history would become a bit messy.

I suspect that Mike's advice about using astyle was more about checking by yourself what kind of reformatting were needed in your patch, and then correct by hand your own code. After astyle --style=linux < infile > outfile, use diff to check for the differences between the two files and update your code accordingly.

Or you may extract your code, apply astyle and then paste back.

Note that I still submit code containing tabs, and by looking at the size of your patch it seems that I am not the only one! Well, we may apply a giant, formatting only, patch latter, and set up our editors to use 8 spaces instead of tabs. But that is a separate issue.

So, if you don't mind sending a new patch limited to your new code, I will be glad to test it!

Bests,

Christophe
Comment 66 Michael Chudobiak 2008-08-14 16:02:45 UTC
Still here for a few more hours...

Yes, the patch should be limited to the new stuff, or it becomes impossible to review.

Tabs and spaces are used interchangeably in gthumb code. Don't worry too much about that - it all looks the same, after all. However, I do care about the bracketing style. 

- Mike
Comment 67 Christophe Bisière 2008-08-14 17:28:16 UTC
(In reply to comment #63)
> Gabriel & Christophe,
> 
> I will be away for ~5 days.
> 
> Christophe, can you review/test/modify Gabriel's patches and merge them into
> your patches, if you are working on gthumb over the next week?
> 
> - Mike
> 

I will do my best.

Christophe
Comment 68 Christophe Bisière 2008-08-16 22:27:38 UTC
Created attachment 116771 [details] [review]
GIO port -- mime

Mike,

This patch converts part of the MIME type detection functions.

- I started with a copy of file-utils.c:get_file_mime_type () in file-roller. I figured out it assumed that g_file_info_get_content_type () returns the value of attribute G_FILE_ATTRIBUTE_STANDARD_FAST_CONTENT_TYPE when g_file_query_info () asked for it. A test showed me that it is not the case. Thus, if I did not make a mistake somewhere, I think file-roller's source code should be corrected.
- I tested with sample image files how the exceptions hard-coded in file-utils:get_file_mime_type () (Nikon nef, Canon cr2, OpenEXR exr, ...) were now handled by GIO, and left only what was necessary.

Also:

- private functions in gfile-utils are now all prefixed by _gfile.
- mute a compile warning triggered by gfile_warning ()

Bests,

Christophe

PS: Thanks for your funny (and quite embarrassing) rev 2400!
Comment 69 Christophe Bisière 2008-08-17 21:13:41 UTC
Created attachment 116831 [details] [review]
convert MIME and use output stream in web exporter

Mike,

This is a new, extended version of my previous patch.

The web exporter now uses output streams.

Note that I had to touch your gfile_output_stream_write_line (). I needed a gfile_output_stream_write () which did not write a "\n", and was able to set its error parameter.

I think the new gfile_output_stream_write_line () is more homogeneous with the stock gio functions: 1) you can pass it an **error or NULL, 2) if error is not NULL, *error will be set if an error occurs, 3) even if you pass NULL, the value returned by the function enables you to determine if an error occurred.

I hope this change is OK for you.

Bests,

Christophe
Comment 70 Gabriel Falcão 2008-08-19 06:00:13 UTC
Created attachment 116930 [details] [review]
Sending files to trash

This patch ports the dlg_files_trash to GIO.
Now i am porting the structure that sends a whole directory to trash.
Comment 71 Christophe Bisière 2008-08-19 07:11:45 UTC
Thanks Gabriel!

The first step to review a patch is to read it directly, to get a sense of what material is added / deleted. In that respect your patch is very difficult to review because most of it is tabs / spaces conversions.

Please try to work out a patch limited to the new stuffs. Adjust your editor's settings such that it does not convert tabs.

Nonetheless I spotted that you create:

  new_uri_string_from_path ()
  get_trash_path ()
  get_trash_uri ()

in libgthumb/file-utils.c, but it seems that you don't use them (which is OK, see comment #55 and comment #56).

Bests,

Christophe
Comment 72 Christophe Bisière 2008-08-19 08:43:57 UTC
Created attachment 116939 [details] [review]
MIME & stream

In this new version, the gthtml parser uses GIO.

Run src/make-albumtheme.sh before make.

Bests,

Christophe
Comment 73 Gabriel Falcão 2008-08-20 13:58:55 UTC
Hi Chris,
i really got some indentation problems, and i've realized that when i used that indentation tool of comment #62, the code got many modifications nonsense.
I am very sorry :(
This week i'll stop working on gthumb because i moved to other city/state, new job, etc.
But i have created a function to send whole folders to trash asyncronnously, i hope to send this patch today.

Regards,
Gabriel
Comment 74 Michael Chudobiak 2008-08-20 19:50:15 UTC
mime/stream patch committed to 2402.

Thanks, Christophe!

- Mike
Comment 75 Michael Chudobiak 2008-08-21 16:51:45 UTC
To help things along, I ran astyle on gfile-utils.c in trunk.

Gabriel, please run "svn update" and "svn diff" again to regenerate your patches. They should be much smaller now, since both sides now have the astyle tidy-ups.

- Mike
Comment 76 Michael Chudobiak 2008-08-28 11:56:52 UTC
Hi Gabriel,

Nudge, nudge... do you have a tidied-up patch for us to play with? :-)

- Mike
Comment 77 Gabriel Falcão 2008-08-30 19:19:22 UTC
Created attachment 117658 [details] [review]
trash stuff done

Finished to port the trash stuff.
Comment 78 dynamotwain 2008-08-31 20:57:06 UTC
Created attachment 117721 [details] [review]
Fix g_propogate_warnings assertion when there is no error.

g_propagate_error should only be called if an error has actually occurred, otherwise an assertion gets thrown when debugging with the G_DEBUG=fatal_warnings environment variable.
Comment 79 Michael Chudobiak 2008-09-04 14:35:20 UTC
Gabriel,

The trash patch is impossible to review. As I've said before, please strip the patch down to its essentials, removing the white space changes!

You can do something like checking out another copy of the code from trunk, running astyle on the affected files (without your patches), and run "diff" (rather than "svn diff") against the astyle+patched files, so that only the "real" changes are written to the patch. (There are probably other methods, but that should work fine.)

I'd love to get your code into trunk, but you are making it hard by ignoring the white space problem.

- Mike
Comment 80 Michael Chudobiak 2008-09-04 14:41:03 UTC
g_propagate_error patch applied to svn rev 2407 (after fixing the bracket style). Thanks, Geoffrey!

- Mike
Comment 81 Gabriel Falcão 2008-09-04 22:00:45 UTC
Mike, i am very sorry about that whitespace issues. I am quite noob on it, but i will follow your tip.

Actually i am not ignoring because i want to. :(

I will send a cleaned version today.

Regards,
Gabriel Falcão
Comment 82 Gabriel Falcão 2008-09-05 03:49:32 UTC
Created attachment 118070 [details] [review]
Trash stuff done and stripped

I did this patch using:
svn diff --diff-cmd diff -x -uw
instead of simply svn diff
Comment 83 Michael Chudobiak 2008-09-10 19:27:03 UTC
Thanks, Gabriel, that is much easier to read! Here are a few minor things to fix, based on a first reading:

1)  /* TODO: g_error_free (error); */  ???

2)  In _after_each_file_trashed, does the string "Copying file %d of %d" make sense, or should it be "Deleting file %d of %d"?

3)  "_after_each_fof_trashed" - What's a "fof"? Where is this function used?

4)  g_message ("Aumentando o index e o total"), and others. In English, please.

5)  gfile_trash_async (GList *file_list,
		   GCancellable *cancel,
		   GFileTrashedCb each_file_callback,
		   gpointer each_file_data,
		   gint io_priority,
		   GSourceFunc done_func,
		   gpointer done_data,
		   GError **error)

  Please follow the gthumb coding style, with the variable names aligned in the same column. (Needs fixing in quite a few spots.)


I haven't actually tested the patch.

- Mike
Comment 84 Gabriel Falcão 2008-09-11 02:19:26 UTC
Created attachment 118490 [details] [review]
Trash stuff done, stripped and checked

Thanks Mike, i hope that it is done now...


About your questions...
1)  /* TODO: g_error_free (error); */  ???
>>> it was just a test i forgot to remove...

2)  In _after_each_file_trashed, does the string "Copying file %d of %d" make
sense, or should it be "Deleting file %d of %d"?
>>> of course, but i was actually just wondering that i should not add new strings to be internationalized and the trash stuff from before used exactly that string. Anyway, i changed to "Deleting ...."
3)  "_after_each_fof_trashed" - What's a "fof"? Where is this function used?
>>> Sorry, my bad. I have removed it this time.
4)  g_message ("Aumentando o index e o total"), and others. In English, please.
>>> Sorry, my bad again... Removed!

5)  gfile_trash_async (GList *file_list,
                   GCancellable *cancel,
                   GFileTrashedCb each_file_callback,
                   gpointer each_file_data,
                   gint io_priority,
                   GSourceFunc done_func,
                   gpointer done_data,
                   GError **error)
>>> Done!
Comment 85 Michael Chudobiak 2008-10-08 19:47:14 UTC
Gabriel,

Sorry for the delay in reviewing. I have less free time these days. Anyway, the patch needs some tweaking. I compile everything with the -Wall flags (./autogen.sh --prefix=/usr CFLAGS="-Wall -ggdb") to catch stuff like this:


[mjc@xena trunk]$ make > eraseme
file-utils.c: In function 'copy_file_async_done':
file-utils.c:2409: warning: control reaches end of non-void function
file-utils.c: At top level:
file-utils.c:76: warning: '_empty_file_progress_cb' defined but not used
gfile-utils.c: In function '_gfile_trashed_cb':
gfile-utils.c:262: warning: control reaches end of non-void function
gfile-utils.c: In function '_gfile_trash_recursive':
gfile-utils.c:281: warning: value computed is not used
gfile-utils.c:282: warning: value computed is not used
gfile-utils.c:315: warning: value computed is not used
gfile-utils.c: In function '_gfile_trash_folder_job_do':
gfile-utils.c:349: warning: control reaches end of non-void function
gfile-utils.c: At top level:
gfile-utils.c:112: warning: '_async_xfer_data_ref' defined but not used
gfile-utils.c:336: warning: '_gfile_trash_folder_job_do' defined but not used
dlg-file-utils.c: In function ‘_trash_files_done’:
dlg-file-utils.c:1761: warning: unused variable ‘fcdata’
dlg-file-utils.c: In function ‘dlg_folder_move_to_trash’:
dlg-file-utils.c:2455: warning: unused variable ‘scan’
dlg-write-to-cd.c: In function ‘ok_clicked_cb’:
dlg-write-to-cd.c:108: warning: passing argument 7 of ‘dlg_copy_items’ from incompatible pointer type
dlg-write-to-cd.c:119: warning: passing argument 7 of ‘dlg_copy_items’ from incompatible pointer type
dlg-write-to-cd.c:131: warning: passing argument 7 of ‘dlg_folder_copy’ from incompatible pointer type
gth-browser.c: In function ‘image_list_drag_data_received’:
gth-browser.c:3874: warning: passing argument 7 of ‘dlg_copy_items’ from incompatible pointer type
gth-browser.c: In function ‘dir_list_drag_data_received’:
gth-browser.c:3952: warning: passing argument 7 of ‘dlg_copy_items’ from incompatible pointer type
gth-browser-actions-callbacks.c: In function ‘folder_delete__continue’:
gth-browser-actions-callbacks.c:1153: warning: passing argument 3 of ‘dlg_folder_delete’ from incompatible pointer type
gth-browser-actions-callbacks.c: In function ‘folder_delete’:
gth-browser-actions-callbacks.c:1195: warning: passing argument 3 of ‘dlg_folder_move_to_trash’ from incompatible pointer type
gth-browser-actions-callbacks.c: In function ‘folder_copy__response_cb’:
gth-browser-actions-callbacks.c:1333: warning: passing argument 7 of ‘dlg_folder_copy’ from incompatible pointer type
catalog-web-exporter.c: In function 'export__copy_to_destination__step2':
catalog-web-exporter.c:2151: warning: passing argument 3 of 'dlg_folder_delete' from incompatible pointer type
catalog-web-exporter.c: In function 'export__copy_to_destination':
catalog-web-exporter.c:2169: warning: passing argument 7 of 'dlg_folder_copy' from incompatible pointer type
catalog-web-exporter.c: In function 'load_next_file':
catalog-web-exporter.c:2529: warning: passing argument 3 of 'dlg_folder_delete' from incompatible pointer type


Could you fix these warnings, please?


- Mike
Comment 86 Gabriel Falcão 2008-10-12 23:27:19 UTC
Yeah, i'll do this as soon as possible.
Comment 87 Michael Chudobiak 2008-11-07 18:09:21 UTC
Hi Gabriel,

Could you take a look at that soon?

- Mike
Comment 88 Michael Chudobiak 2008-12-19 13:08:23 UTC
Gabriel, Christophe, mclasen, anybody...

Can someone donate some spare time to get the gio patches rolling again? We've stalled badly...

- Mike
Comment 89 Michael Chudobiak 2008-12-19 13:11:45 UTC
*** Bug 560696 has been marked as a duplicate of this bug. ***
Comment 90 Christophe Bisière 2008-12-20 16:26:00 UTC
Hi Mike,

Indeed, it's a pity.

Working on the GIO port is still on my todo list. Unfortunately, I have a hard time finding time to work on it. I expect Q1 2009 to be a bit better with that respect, though.

Best regards,

Christophe
Comment 91 Michael Chudobiak 2008-12-21 12:07:23 UTC
OK, maybe someone can give me some gvfs pointers here.

I've added a FileData->local_path entry to point to the ~/.gvfs mount of remote files (and the normal path of local files). This lets us replaces the local_file cache that we had been using, which simplifies a lot of things.

The path is determined in libgthumb/file-utils.c:gfile_get_path_from_uri.

I have a g_assert to abort if the returned path is NULL.

However, the problem I'm having is that to access, say:

sftp://server/foo/bar.jpg

I have to open it first in Nautilus, otherwise if I try to open it in gthumb (from a bookmark, say) this assertion trips.

So I must be missing a "mount" step in gthumb. Is that right?

In other words, for an arbitrary and existing gio uri, how do I ensure that the matching ~/.gvfs file exists?

- Mike
Comment 92 Michael Chudobiak 2008-12-22 10:27:08 UTC
Never mind my last comment - I was looking for g_file_mount_enclosing_volume & g_file_mount_enclosing_volume_finish.

However, I reverted my changes for now - too many things had to be changed (or broken) at once...

- Mike
Comment 93 A. Walton 2008-12-26 11:20:44 UTC
> In other words, for an arbitrary and existing gio uri, how do I ensure that the
> matching ~/.gvfs file exists?

You can't really assure the FUSE mounted path exists, period. Some users won't run the FUSE daemon, some administrators don't want it running on their systems, etc. If it's there, great, if it's not, you're going to have to deal with that too.

We also (AFAIK) never merged the patch in 530654 for reverse mapping GDaemonFiles to fuse paths, so that's the reason for g_file_get_path() returning NULL.
Comment 94 Michael Chudobiak 2008-12-27 19:07:43 UTC
OK, I've purged a lot of gnomevfs code from trunk lately. Still lots to do :-(

Is there anyone interested who could take a look at:

1) Tidying and testing Gabriel's orphaned patch
2) Porting libgthumb/file-utils.c:resolve_symlinks to use GFile
3) Porting src/gth-monitor.c to use GFileMonitor
4) "grep -i vfs */*.c" and fixing anything they see!

- Mike
Comment 95 Christophe Bisière 2008-12-28 12:46:02 UTC
Mike,

If Gabriel does not mind, I am going to work on 1). It will help me to get rid of the last pieces of GnomeVFS in catalog_web_exporter.c.

--Christophe
Comment 96 Michael Chudobiak 2008-12-28 23:19:23 UTC
Great, Christophe!

I'll be away from my computer for a week or so...

- Mike
Comment 97 Christophe Bisière 2009-01-03 18:36:54 UTC
Created attachment 125696 [details] [review]
isolate vfs code in dlg-file-utils.c

Migrating dlg-file-utils.c and file-utils.c is going to be tricky, so we should proceed carefully.

Here is a baby step!

This patch isolates the old VFS code in dlg-file-utils.c, preparing its replacement by GIO code. It generalizes something Gabriel started in his patch.

In particular, the attached patch completely replaces GnomeVFSResult members in struct FolderCopyData and friends by GError members.

Also, the function set_error_from_vfs_result () maps VFS error codes to GIO GErrors. It is meant to be deleted when the port of dlg-file-utils.c is done.

Best Regards and Happy New Year!

Christophe
Comment 98 Michael Chudobiak 2009-01-05 15:43:16 UTC
Hi Christophe!

I'm back now, and I'll review the patch in the next day or two.

- Mike
Comment 99 Michael Chudobiak 2009-01-05 20:51:23 UTC
Patch committed, thanks!

One addition - I deleted:

-#include <libgnomevfs/gnome-vfs-utils.h>

from src/dlg-write-to-cd.c

- Mike
Comment 100 Michael Chudobiak 2009-03-23 17:09:03 UTC
Christophe,

Any further patches? Nudge, nudge...  :-)

- Mike
Comment 101 Christophe Bisière 2009-03-24 09:43:31 UTC
Mike,

Unfortunately, not yet.

This is not due to a lack of interest for the project. Just a lack of free time...

Keep nudging me from time to time!

8-)

Christophe
Comment 102 Michael Chudobiak 2009-03-30 11:42:38 UTC
Another gio todo: the photo importer should use the gphoto/gio backend end, if present, and then fall back to the existing libgphoto code otherwise.

Otherwise, the gvfs code mounts the camera and breaks the libgphoto-based access.

- Mike
Comment 103 Michael Chudobiak 2009-04-23 19:59:13 UTC
I've put in a whole bunch of commits recently. We are getting closer!

Can someone take a look at gfiling path_list_async_new? (The sync version of it already exists.) It's a nicely isolated bit of code. I'm just not sure that I understand how to cancel async gfile operations properly (and whether the existing gnomevfs code is doing it properly either).

I'll be away for a few days...

Note: To checkout gthumb, you now need git:

git clone git://git.gnome.org/gthumb

- Mike
Comment 104 Marlodavampire 2009-05-28 23:50:47 UTC
I'm currently working on this bug.
Comment 105 Marlodavampire 2009-06-02 02:03:49 UTC
Created attachment 135783 [details] [review]
GIO conversion of path_list_async_new and associated functions.

GIO conversion of path_list_async_new and associated functions.
Move slow file classify code into an idle callback
Implement gcancellable for path list code
Remove GnomeVfsResult from PathListData
comments/formatting
Comment 106 Marlodavampire 2009-06-02 02:06:22 UTC
Created attachment 135784 [details] [review]
GIO root path (Filesystem) string updates

Add a special case to remove_ending_separator for root dir
Change File System elements uri to file:///
Update file:/// string in get_icon_for_uri
Add checks for file:// vs file:/// to update_uri()
Comment 107 Michael Chudobiak 2009-06-02 14:40:37 UTC
The path_list_async_new patch is committed, with minor formatting adjustments, like:

if( foo( bar ))

changes to

if (foo (bar))

Thanks!

- Mike
Comment 108 Michael Chudobiak 2009-06-02 14:53:58 UTC
I've committed the root-path patch, but some things are still broken.

Running "gthumb /" shows the filesystem children, but running "gthumb /home" and then clicking on ".." does not.

- Mike
Comment 109 Michael Chudobiak 2009-06-02 14:55:35 UTC
The gth-location code should make use of g_file_get_parent anyway, once it is migrated, instead of remove_level_from_path.

- Mike
Comment 110 Marlodavampire 2009-06-02 15:43:19 UTC
Created attachment 135820 [details] [review]
GIO Fix .. element in location sidebar
Comment 111 Michael Chudobiak 2009-06-02 18:03:29 UTC
That doesn't look right... you have:

g_strdup("file:///");

The duplicated string isn't assigned to anything.

- Mike
Comment 112 Marlodavampire 2009-06-03 03:38:31 UTC
Created attachment 135860 [details] [review]
GIO Fix .. element in location sidebar

Oops. Should be assigned to new_path.
Comment 113 Michael Chudobiak 2009-06-03 13:38:23 UTC
Committed!
Comment 114 Marlodavampire 2009-06-03 16:32:05 UTC
Created attachment 135887 [details] [review]
GIO src/gth-location.c converstion

update_drives converted
update_uri converted
monitor_changed_cb converted
Unused functions/includes removed

This needs lots of testing with device mounting and unmounting and catalgoue browsing.
Comment 115 Michael Chudobiak 2009-06-03 16:41:46 UTC
You attached the wrong patch.
Comment 116 Marlodavampire 2009-06-03 16:45:58 UTC
Created attachment 135889 [details] [review]
GIO src/gth-location.c converstion

Indeed, how careless.

update_drives converted
update_uri converted
monitor_changed_cb converted
Unused functions/includes removed
Comment 117 Michael Chudobiak 2009-06-03 17:05:18 UTC
Some quick comments:

1) Please run "astyle --style=linux src/gth-location.c" before submitting the patch, just to tidy things up a bit.

2) If I mount a floppy, the floppy appears in the volume list, but it has no icon.

3) If I mount a sftp:// uri in Nautilus, I can open with gthumb (from Nautilus), but the sftp:// mount does not show up in the volume list. It should.

I'll have to test it more later, of course.

- Mike
Comment 118 Michael Chudobiak 2009-06-03 18:20:12 UTC
4) If I try to load a bookmark for a location that no longer exists (like a folder that has been deleted, or a remote URI that is no longer mounted), no error is thrown. The directory appears to load. An error dialog should be launched instead, and the directory should not change.
Comment 119 Michael Chudobiak 2009-06-03 19:02:28 UTC
I've committed the above patch, with minor formatting tweaks, to get more experience with it.

Thanks!

- Mike
Comment 120 Marlodavampire 2009-06-04 04:11:53 UTC
Created attachment 135919 [details] [review]
GIO Fix error passing on directory load

Replace GnomeVFSResult with GError for dir_list_done_cb, gth_dir_list_change_to
__step2 and directory_load_cb
Comment 121 Marlodavampire 2009-06-04 04:31:41 UTC
Created attachment 135920 [details] [review]
GIO Fix error passing on directory load

GIO Fix error passing on directory load
Replace GnomeVFSResult with GError for dir_list_done_cb, gth_dir_list_change_to__step2 and directory_load_cb
Remove unused GnomeVFS includes
Comment 122 Marlodavampire 2009-06-04 12:03:43 UTC
Created attachment 135930 [details] [review]
GIO Fixes to allow remote mount browsing

Change path_list_classify_files_cb to use g_file_enumerator_get_next_file_async
Remove local file restriction from update_drives
Comment 123 Michael Chudobiak 2009-06-04 12:06:23 UTC
If you try to load a bookmark for a location that no longer exists, you do not get an error dialog. These warnings occur on the console:

(gthumb:13007): GLib-GObject-CRITICAL **: g_value_get_int: assertion `G_VALUE_HOLDS_INT (value)' failed

I would guess that "gthumb_marshal_VOID__INT" is no longer appropriate in g_signal_new ("done",...).

So the patch in comment 121 needs some fixing...

- Mike


Comment 124 Marlodavampire 2009-06-04 13:13:37 UTC
Created attachment 135935 [details] [review]
GIO Fix error passing on directory load

Replace GnomeVFSResult with GError for dir_list_done_cb,
gth_dir_list_change_to__step2 and directory_load_cb
Remove unused GnomeVFS includes

I can't reproduce the problem but I think your right about g_signal_new, please try this.
Comment 125 Michael Chudobiak 2009-06-04 13:42:18 UTC
OK, both committed. One glitch:

1. Mount a remote "sftp://..." location in nautilus.

2. Go to it in gthumb, view some photos, bookmark a location (under sftp://...)

3. Unmount the remote URI in nautilus.

4. Try to load the remote bookmark in gthumb. gthumb freezes.

- Mike
Comment 126 Michael Chudobiak 2009-06-04 14:13:11 UTC
The icon-related stuff in src/main.c should probably be moved in gth-location.c. Also, can we rely more on gfile to provide icons?

- Mike
Comment 127 Marlodavampire 2009-06-04 15:32:48 UTC
Created attachment 135948 [details] [review]
GIO Fix bookmarks to unmounted locations
Comment 128 Marlodavampire 2009-06-04 15:43:26 UTC
It should be possible to get all our icons through GFile (and GMount/GVolume/GDrive) functions in the future. src/gth-location.c get_mount_icon() already gets an icon for a GMount but it doesn't seem to work for new mounts (e.g. inserting a CD). Possibly because the mount isn't ready yet when its run, I haven't looked into it. 

Maybe you can open a new bug for icon changes?
Comment 129 Michael Chudobiak 2009-06-06 19:00:13 UTC
The icons are probably working better now, after I committed a change to the way gicons were converted into pixbufs.

- Mike
Comment 130 Marlodavampire 2009-06-17 00:47:17 UTC
Created attachment 136789 [details] [review]
GIO use ITEMS_PER_NOTIFACTION for g_file_enumerator_next_files_async
Comment 131 Marlodavampire 2009-06-17 00:54:58 UTC
(In reply to comment #130)
> Created an attachment (id=136789) [edit]
> GIO use ITEMS_PER_NOTIFACTION for g_file_enumerator_next_files_async
> 

Please ignore this, it's right the way it is, this patch will break things on large directory lists.
Comment 132 Marlodavampire 2009-06-18 06:09:44 UTC
Created attachment 136891 [details] [review]
GIO conversion of dlg-duplicates to GFile

Rewrite search_dir_async and search_dir_next_files_cb using GFile
Add search_dir_next_files_cb function
Use GCancellable for canceling the search
Change data->dirs and data->queue to a GFile GList
Add a GFile to ImageData
Convert file reading functions to GFile
Update get_file_mtime and add gfile_get_mtime
Add gfile_is_image_video_or_audio function
Comment 133 Michael Chudobiak 2009-06-18 12:08:30 UTC
Thanks! Committed with some revisions.

- Mike
Comment 134 Marlodavampire 2009-06-22 06:18:53 UTC
Created attachment 137151 [details] [review]
GIO dlg-search conversion

Add start_from_gfile to SearchData
Pass name_only to file_respects_search_criteria
Update search_dir_aysnc to use GFiles
Update directory_load_cb to use GFiles
Add scan_next_dir function
Add search_dir_next_files_cb
Change data->dirs to GFiles


Unlike dlg-duplicates, dlg-search follows symlinks so please remember to test with some symlinks.
Comment 135 Michael Chudobiak 2009-06-24 13:46:26 UTC
Committed with minor revisions. Thanks!

- Mike
Comment 136 Marlodavampire 2009-06-25 03:23:37 UTC
Created attachment 137349 [details] [review]
GIO Error handling

Remeber to call g_error_free on errors in a few places
Fix a mistake with error handling in path_list_next_files_cb
Only request standard file attributes in path_list_async_new
Pass the right gfile to gfile_warning in directory_load_cb
Improve error handling for symlinks in dlg-search
Comment 137 Marlodavampire 2009-07-03 12:32:03 UTC
Created attachment 137785 [details] [review]
GIO port dlg-file-utils delete and trash functions to GFile

Add get_checked_images_as_fd to dlg-duplicates
Update delete_cb in dlg-duplicates to call dlg_file_delete__confirm with a FileData GList
Rewrite dlg_file_delete__confirm and real_files_delete(_continue) functions to use GFiles.
Add real_files_delete__no_trash
Rewrite dlg_files_move_to_trash and dlg_files_delete using GFile
Add delete_next_file and trash_next_file_function
Add new function for displaying progress dialog while trashing files
Update gth_browser_activate_action_image_delete to call dlg_file_delete__confirm with a FileData GList
Update gth-fullscreen.c delete_current_image to call dlg_file_delete__confirm with a FileData GList

There is a new string in file_trash_progress_update, I'm not sure what needs to be done for translations.

This needs more testing, particularly on large file sets and on sets that stop because of error (for e.g. no permission to delete a file in the set).
Comment 138 Michael Chudobiak 2009-07-03 12:42:47 UTC
Nothing special needs to be done for translation except:

1. Mark the string for translation using the _("") macro.

2. Make sure the file is listed in po/POTFILES.in.

- Mike
Comment 139 Marlodavampire 2009-07-08 18:29:33 UTC
That should already be done.

Anything else need fixing or changing before you're happy with this? It's kind of blocking the rest of the work on dlg-file-utils until this is committed/merged in a fairly stable state.
Comment 140 Michael Chudobiak 2009-07-08 18:43:52 UTC
Sorry, I've been slow and busy :-(

I didn't see any major issues on a first read-through. Just the usual formatting annoyances.

I'll try to commit it "soon".

- Mike
Comment 141 Marlodavampire 2009-07-08 19:01:49 UTC
No problem. As long as you don't think any major changes will be required I'll keep going with the other functions in dlg-file-utils.

I'm not sure how you work with Git but something I would really appreciate if you can do it is, when fixing up my formatting etc. do it as two commits. First with my patch unchanged then with your changes as anther commit. It seems to be the way recommended by most people and it would help me easily see what changes you're making.
Comment 142 Michael Chudobiak 2009-07-08 19:13:33 UTC
OK, that sounds like a good idea.

I'll try to get to it tonight.

- Mike
Comment 143 Michael Chudobiak 2009-07-09 01:14:21 UTC
I committed the patch, with a follow-up commit to fix minor style issues.

However, bug 588125 needs to be fixed - there is some improper unref'ing in the duplicates tool.

- Mike
Comment 144 Marlodavampire 2009-07-15 22:40:43 UTC
Created attachment 138486 [details] [review]
GIO port dlg-file-utils to GFile

Complete rewrite of folder_copy functions using GIO/GFile
Convert file_copy functions to GIO/GFile
Convert item_copy functions to use the new file_copy/folder_copy functions
Update dlg-write-to-cd, gth-browser-action-callbacks, catalog-web-exporter to use the new functions
Update overwrite_dialog functions
Removed all remaining GnomeVFS functions/references from dlg-file-utils

There is a lot of completely new code here, needs testing with copying/moving files and folders, dragging & dropping files/folders and deleting folders.
Comment 145 Michael Chudobiak 2009-07-16 19:06:38 UTC
Bug 587795 has broken my build system (anybody know how to fix it?), and I will be away next week, so I will be slow to review this patch...

- Mike
Comment 146 Javier Jardón (IRC: jjardon) 2009-09-05 19:39:02 UTC
Is this bug fixed in the ext branch?
Comment 147 Paolo Bacchilega 2009-09-06 07:32:57 UTC
yes