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

Adrian Perez
Reported 2016-07-29 16:18:01 PDT
An implementation of WebCore::moveFile() is not available for the GTK+ port.
Attachments
Patch (1.50 KB, patch)
2016-07-29 16:20 PDT, Adrian Perez 		no flags
DetailsFormatted DiffDiff
Patch (1.53 KB, patch)
2016-07-30 02:33 PDT, Adrian Perez 		no flags
DetailsFormatted DiffDiff
Patch (1.53 KB, patch)
2016-07-31 15:28 PDT, Adrian Perez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Adrian Perez
Comment 1 2016-07-29 16:20:31 PDT
Created attachment 284914 [details] Patch
Carlos Garcia Campos
Comment 2 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
Carlos Garcia Campos
Comment 3 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.
Adrian Perez
Comment 4 2016-07-30 02:33:01 PDT
Created attachment 284938 [details] Patch
Adrian Perez
Comment 5 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>
Michael Catanzaro
Comment 6 2016-07-30 09:07:48 PDT
prepare-ChangeLog -b <BUGID>
Carlos Garcia Campos
Comment 7 2016-07-31 01:13:11 PDT
(In reply to comment #6) > prepare-ChangeLog -b <BUGID> prepare-ChangeLog -g HEAD -b <BUGID>
Carlos Garcia Campos
Comment 8 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
Adrian Perez
Comment 9 2016-07-31 15:28:08 PDT
Created attachment 284971 [details] Patch
Adrian Perez
Comment 10 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.
WebKit Commit Bot
Comment 11 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>
WebKit Commit Bot
Comment 12 2016-07-31 22:45:18 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.