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 773671 - Offers to replace directory with file
Offers to replace directory with file
Status: RESOLVED FIXED
Product: nautilus
Classification: Core
Component: File and Folder Operations
3.26.x
Other Linux
: Normal normal
: 3.26
Assigned To: Nautilus Maintainers
Nautilus Maintainers
: 425980 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2016-10-29 14:00 UTC by Bastien Nocera
Modified: 2017-10-28 12:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
screenshot (41.89 KB, image/png)
2016-10-29 14:01 UTC, Bastien Nocera
  Details
operations-ui: check and handle file on folder copy. (3.58 KB, patch)
2017-05-06 12:26 UTC, Vyas Giridhar
none Details | Review
operations-ui: check and handle file on folder copy. (3.74 KB, patch)
2017-07-21 03:24 UTC, Vyas Giridhar
rejected Details | Review
file-operations: fail when overwriting directory with file (6.48 KB, patch)
2017-08-28 14:53 UTC, Ernestas Kulik
none Details | Review
file-operations: fail when overwriting directory with file (6.76 KB, patch)
2017-08-28 16:12 UTC, Ernestas Kulik
committed Details | Review

Description Bastien Nocera 2016-10-29 14:00:41 UTC
Follow-up from bug 773670

I don't know how that would work, but nautilus offers to replace the directory with a file when they have the same name.

In the example, directory gbiblio.m7 contains a file called gbiblio.m7. Trying to move the file to the parent directory throws the error attached. Selecting "Replace" will simply fail.
Comment 1 Bastien Nocera 2016-10-29 14:01:03 UTC
Created attachment 338774 [details]
screenshot
Comment 2 Vyas Giridhar 2017-05-06 12:26:34 UTC
Created attachment 351256 [details] [review]
operations-ui: check and handle file on folder copy.

nautilus offers to replace the directory with a file when they have
the same name.
for example, directory gbiblio.m7 contains a file called gbiblio.m7.
Trying to move the file gbiblio.m7 to the parent directory throws the
error attached. Selecting "Replace" will simply fail.

This patch adds a error dialog and cancels the operation
Comment 3 Carlos Soriano 2017-05-30 15:34:19 UTC
Review of attachment 351256 [details] [review]:

Hey Vyas, I like the solution as it simple. However there are some points to take care:

::: src/nautilus-operations-ui-manager.c
@@ +489,3 @@
+                                    GTK_MESSAGE_ERROR,
+                                    GTK_BUTTONS_CANCEL,
+    int response_id;

I think it's better to not use "strong" entonations with exclamations for errors. So I think it's better if we remove the exclamation symbol.

@@ +517,3 @@
     data->destination_directory_name = destination_directory_name;
 
+    data->source = nautilus_file_get (data->source_name);

we don't use nautilus cache on pourpose in file operations. That requires to poke the disk before hand with the *call_when_ready operations. In here you would need to use directly GFile operations.

@@ +537,3 @@
+    {
+        invoke_main_context_sync (NULL,
+        invoke_main_context_sync (NULL,

maybe this dialog should have a parameter to wheter the problem is folder vs file replacement? So we can use it on other conflicts, other than here, and we avoid another function and more code.
Comment 4 Vyas Giridhar 2017-07-21 03:24:56 UTC
Created attachment 356079 [details] [review]
operations-ui: check and handle file on folder copy.

operations-ui: check and handle file on folder 

nautilus offers to replace the directory with a file when they have
the same name.
for example, directory gbiblio.m7 contains a file called gbiblio.m7.
Trying to move the file gbiblio.m7 to the parent directory throws the
error attached. Selecting "Replace" will simply fail.
 
This patch adds a error dialog and cancels the operation
Comment 5 Ernestas Kulik 2017-07-24 09:50:26 UTC
Review of attachment 356079 [details] [review]:

I don’t agree with this solution, because this is working around the fact that the operations were written in a way that allows overwriting directories with regular files.
There is a simpler way to do this, but the result is an uglier dialog.

The problem there is twofold: copying or moving a file over a directory results in G_IO_ERROR_EXISTS (when the overwrite flag is unset), so what works in copy_move_file() and move_file_prepare() is setting it and retrying, because then you get G_IO_ERROR_IS_DIRECTORY, which is what we want to display the error, but which is, again, special cased in the operation code (just a removal of a couple of lines).

::: src/nautilus-operations-ui-manager.c
@@ +531,3 @@
+                                                NULL);
+    GFileInfo *destination_info = g_file_query_info (destination_name,
+                                                     "standart::display-name, standard::type",

Better to use GIO definitions for attribute strings to avoid typos like this.
Comment 6 Ernestas Kulik 2017-08-18 10:54:56 UTC
Comment on attachment 356079 [details] [review]
operations-ui: check and handle file on folder copy.

Changing the status of the patch to reduce the noise in the list of reviewed patches.
Comment 7 Ernestas Kulik 2017-08-28 14:53:52 UTC
Created attachment 358603 [details] [review]
file-operations: fail when overwriting directory with file

GIO does not allow that and for a good reason.
Comment 8 Carlos Soriano 2017-08-28 15:37:37 UTC
Review of attachment 358603 [details] [review]:

The most important part of this patch is the reasoning, and it's what is missing. Could you expand on the commit message?

The code looks good!
Comment 9 Carlos Soriano 2017-08-28 15:59:28 UTC
Review of attachment 356079 [details] [review]:

rejected in favour of Ernestas patch that removes support for overwriting directories with files.
Comment 10 Carlos Soriano 2017-08-28 15:59:28 UTC
Review of attachment 356079 [details] [review]:

rejected in favour of Ernestas patch that removes support for overwriting directories with files.
Comment 11 Ernestas Kulik 2017-08-28 16:12:05 UTC
Created attachment 358608 [details] [review]
file-operations: fail when overwriting directory with file

GIO documentation (and therefore code) mandates that copies and moves of
files over directories should fail, but Nautilus has support for such
(possibly dangerous) operations even to this day. This commit works with
the assumption that whatever backend happens to be used, it will fail as
expected and not overwrite the directory.
Comment 12 Carlos Soriano 2017-08-28 16:21:54 UTC
Review of attachment 358608 [details] [review]:

Looks good now, thanks Ernestas!
Comment 13 Ernestas Kulik 2017-08-28 16:50:19 UTC
Attachment 358608 [details] pushed as 1349a35 - file-operations: fail when overwriting directory with file
Comment 14 António Fernandes 2017-10-28 12:02:26 UTC
*** Bug 425980 has been marked as a duplicate of this bug. ***