Bug 160363

Summary: [GTK] Implement missing WebCore::moveFile() using GLib functions
Product: WebKit Reporter: Adrian Perez <aperez>
Component: WebKitGTKAssignee: Adrian Perez <aperez>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, beidson, bugs-noreply, cgarcia, commit-queue, darin, mcatanzaro
Priority: P2    
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 154553    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Adrian Perez 2016-07-29 16:18:01 PDT 
An implementation of WebCore::moveFile() is not available for the GTK+ port.
Comment 1 Adrian Perez 2016-07-29 16:20:31 PDT 
Created attachment 284914 [details]
Patch
Comment 2 Carlos Garcia Campos 2016-07-29 23:50:00 PDT 
Comment on attachment 284914 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=284914&action=review

Thanks!

> Source/WebCore/platform/glib/FileSystemGlib.cpp:381
> +    return !g_rename(oldFilename.get(), oldFilename.get());

g_rename returns -1 in case of failure, I prefer to explicitly check that like all other functions in this file do. return rename != -1
Comment 3 Carlos Garcia Campos 2016-07-29 23:50:46 PDT 
Comment on attachment 284914 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=284914&action=review

> Source/WebCore/ChangeLog:3
> +        Implement WebCore::moveFile() using GLib.

And please, use the actual bug title here. prepare-ChangeLog should do the right thing for you.
Comment 4 Adrian Perez 2016-07-30 02:33:01 PDT 
Created attachment 284938 [details]
Patch
Comment 5 Adrian Perez 2016-07-30 03:52:03 PDT 
An updated patch is now uploaded.

(In reply to comment #3)
> Comment on attachment 284914 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=284914&action=review
> 
> > Source/WebCore/ChangeLog:3
> > +        Implement WebCore::moveFile() using GLib.
> 
> And please, use the actual bug title here. prepare-ChangeLog should do the
> right thing for you.

I couldn't figure out how to make this work, but instead I have discovered
that the following works like a charm:

  % webkit-patch prepare -g HEAD --update-changelogs <BUGID>
Comment 6 Michael Catanzaro 2016-07-30 09:07:48 PDT 
prepare-ChangeLog -b <BUGID>
Comment 7 Carlos Garcia Campos 2016-07-31 01:13:11 PDT 
(In reply to comment #6)
> prepare-ChangeLog -b <BUGID>

prepare-ChangeLog -g HEAD -b <BUGID>
Comment 8 Carlos Garcia Campos 2016-07-31 01:14:45 PDT 
Comment on attachment 284938 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=284938&action=review

> Source/WebCore/platform/glib/FileSystemGlib.cpp:381
> +    return g_rename(oldFilename.get(), oldFilename.get()) != -1;

I didn't notice this in the previous patch, but you are using oldFilename in both arguments! :-D
Comment 9 Adrian Perez 2016-07-31 15:28:08 PDT 
Created attachment 284971 [details]
Patch
Comment 10 Adrian Perez 2016-07-31 15:29:10 PDT 
(In reply to comment #8)
> Comment on attachment 284938 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=284938&action=review
> 
> > Source/WebCore/platform/glib/FileSystemGlib.cpp:381
> > +    return g_rename(oldFilename.get(), oldFilename.get()) != -1;
> 
> I didn't notice this in the previous patch, but you are using oldFilename in
> both arguments! :-D

Ouch, good catch! I have just uploaded a fixed version of the patch.
Comment 11 WebKit Commit Bot 2016-07-31 22:45:14 PDT 
Comment on attachment 284971 [details]
Patch

Clearing flags on attachment: 284971

Committed r203960: <http://trac.webkit.org/changeset/203960>
Comment 12 WebKit Commit Bot 2016-07-31 22:45:18 PDT 
All reviewed patches have been landed.  Closing bug.