Blender file explorer hard deletes data #61412

New Issue

Ludvik Koutny · 2019-02-11T12:43:20+01:00

Ludvik Koutny commented

2019-02-11 12:43:20 +01:00

Operating system: Windows 10
Graphics card: GTX1080Ti

Blender Version
Broken: 2.8

Short description of error
Deleting a file in Blender's file explorer on Windows hard deletes the file. I don't think that's how any Windows app should behave. Common standard for deleting files within all Windows applications I know of should be soft deletion, which means moving the files to recycle bin. One accidental deletion can be completely destructive.

Exact steps for others to reproduce the error
1, On Windows, open Blender
2, In Blender, open File Browser and delete any file or folder

Result: The file has been hard deleted (not present in recycle bin)

Expected: The file has been soft deleted (moved to recycle bin)

Operating system: Windows 10 Graphics card: GTX1080Ti **Blender Version** Broken: 2.8 **Short description of error** Deleting a file in Blender's file explorer on Windows hard deletes the file. I don't think that's how any Windows app should behave. Common standard for deleting files within all Windows applications I know of should be soft deletion, which means moving the files to recycle bin. One accidental deletion can be completely destructive. **Exact steps for others to reproduce the error** 1, On Windows, open Blender 2, In Blender, open File Browser and delete any file or folder Result: The file has been hard deleted (not present in recycle bin) Expected: The file has been soft deleted (moved to recycle bin)

Ludvik Koutny commented

2019-02-11 12:43:20 +01:00

Added subscriber: @Rawalanche

William Reynish commented

2019-02-11 12:46:11 +01:00

Added subscriber: @WilliamReynish

William Reynish commented

2019-02-11 12:46:11 +01:00

Yeah this is really scary. Currently, if you are not careful, you can completely screw up your system from the Blender File Browser. This is not acceptable behaviour.

IMO we should either remove the ability to delete stuff from the File Browser completely, or make it do what you suggest - to use the system Trash / Recycle Bin.

It's not a bug though - but a critical issue. Not sure how to triage.

Yeah this is really scary. Currently, if you are not careful, you can completely screw up your system from the Blender File Browser. This is not acceptable behaviour. IMO we should either remove the ability to delete stuff from the File Browser completely, or make it do what you suggest - to use the system Trash / Recycle Bin. It's not a bug though - but a critical issue. Not sure how to triage.

Ludvik Koutny commented

2019-02-11 13:00:22 +01:00

In #61412#617664, @WilliamReynish wrote:
It's not a bug though - but a critical issue. Not sure how to triage.

Yes, I wasn't sure about this either, but at the same time, I can't imagine putting this on rightclickselect. That's where ideas go to die. I think Blender needs some more official channel for things that are not necessarily new feature requests, but not bugs either. Something called "Issues" with some voting system. That would be ideal place for things like this, or other things that are broken (such as texture baking) but not technically bugged.

> In #61412#617664, @WilliamReynish wrote: > It's not a bug though - but a critical issue. Not sure how to triage. Yes, I wasn't sure about this either, but at the same time, I can't imagine putting this on rightclickselect. That's where ideas go to die. I think Blender needs some more official channel for things that are not necessarily new feature requests, but not bugs either. Something called "Issues" with some voting system. That would be ideal place for things like this, or other things that are broken (such as texture baking) but not technically bugged.

William Reynish commented

2019-02-11 13:00:54 +01:00

I'll mark it as a confirmed bug because of the severity of the danger of this issue. We could consider it a bug that it doesn't use the system Trash, even though it's not a normal bug in the traditional sense.

At the very least, it's misguided behaviour that can cause real destructive harm.

I'll mark it as a confirmed bug because of the severity of the danger of this issue. We could consider it a bug that it doesn't use the system Trash, even though it's not a normal bug in the traditional sense. At the very least, it's misguided behaviour that can cause real destructive harm.

artem ivanov commented

2019-02-11 23:27:26 +01:00

Added subscriber: @ixd

Ray Mairlot commented

2019-02-12 10:07:27 +01:00

Added subscriber: @RayMairlot

Robert Guetzkow commented

2019-02-12 21:22:17 +01:00

Added subscriber: @rjg

Campbell Barton commented

2019-02-14 01:28:43 +01:00

Added subscriber: @ideasman42

Campbell Barton commented

2019-02-14 01:28:43 +01:00

This raises some questions - should a file selector allow destructive operations at all (I quite like that Gnome/GTK file selector is readonly).

Even if this is done, Blender has a file-space type - where renaming/deleting is something we probably want to keep.

Setting as a TODO.

This raises some questions - should a file selector allow destructive operations at all *(I quite like that Gnome/GTK file selector is readonly)*. Even if this is done, Blender has a file-space type - where renaming/deleting is something we probably want to keep. Setting as a TODO.

Ludvik Koutny commented

2019-02-14 08:55:52 +01:00

I vote for ability to retain all the functionality, just changing what delete does from hard delete to moving to recycle bin. I am not familiar how hard it is to implement on Windows, but if it's not too hard, that would be the way to go. All the other Windows software has either its own file browser or uses the Windows one, but in both cases, deleting is expected to mean moving to recycle bin.

Robert Guetzkow commented

2019-03-24 11:57:31 +01:00

Added subscriber: @Metricity-4

Robert Guetzkow commented

2019-03-24 11:57:31 +01:00

@Metricity-4 SHFileOperation is the legacy API which doesn't support paths longer than 255 characters. If XP support is required we could use the VersionHelper to determine the OS version running and use IFileOperation if it's Vista or later. Otherwise I'd go with IFileOperation , because that is the the current API.

@Metricity-4 `SHFileOperation` is the legacy API which doesn't support paths longer than 255 characters. If XP support is required we could use the `VersionHelper` to determine the OS version running and use `IFileOperation` if it's Vista or later. Otherwise I'd go with `IFileOperation` , because that is the the current API.

Robert Guetzkow commented

2019-03-24 12:15:27 +01:00

Looks like Windows made this one a bit more difficult. The FOFX_RECYCLEONDELETE was introduced for Windows 8, so I'm not sure how that functionality could be achieved on Vista using the new API. Perhaps we should use SHFileOperation for Vista, Windows 7 and IFileOperation for newer versions?

Looks like Windows made this one a bit more difficult. The `FOFX_RECYCLEONDELETE` was introduced for Windows 8, so I'm not sure how that functionality could be achieved on Vista using the new API. Perhaps we should use `SHFileOperation` for Vista, Windows 7 and `IFileOperation` for newer versions?

yvt commented

2019-03-24 12:51:44 +01:00

Added subscriber: @TomoakiKawada

Robert Guetzkow commented

2019-03-24 18:09:09 +01:00

I'm doing some test to see if my idea with IFileOperation works.

I'm doing some test to see if my idea with `IFileOperation` works.

Ray molenkamp commented

2019-03-24 18:13:30 +01:00

Added subscriber: @LazyDodo

Ray molenkamp commented

2019-03-24 18:13:30 +01:00

You don't have to worry about XP, we don't support it.

Simon G. commented

2019-03-24 18:49:54 +01:00

Added subscriber: @intrigus-3

Kris commented

2019-03-24 23:13:54 +01:00

I had a quick look at IFileOperation and was unsure regarding the single threaded/multi threaded apartment notes so went with older method, I hadn't considered the path limitation.

Robert Guetzkow commented

2019-03-24 23:38:46 +01:00

@Metricity-4 I have implemented a first version that works, but I'll have to do more testing. The API's documentation is C++ only, which made it a bit difficult. I'm also not sure whether the choice between COINIT_APARTMENTTHREADED vs COINIT_MULTITHREADED makes a big difference. For simplicity I chose COINIT_APARTMENTTHREADED which is what the official examples use and by the description in the documentation it won't cause us any headaches with concurrency.

@Metricity-4 I have implemented a first version that works, but I'll have to do more testing. The API's documentation is C++ only, which made it a bit difficult. I'm also not sure whether the choice between `COINIT_APARTMENTTHREADED` vs `COINIT_MULTITHREADED` makes a big difference. For simplicity I chose `COINIT_APARTMENTTHREADED` which is what the official examples use and by the [description in the documentation](https://docs.microsoft.com/de-de/windows/desktop/api/objbase/ne-objbase-tagcoinit) it won't cause us any headaches with concurrency.

Robert Guetzkow commented

2019-03-25 10:59:48 +01:00

@Metricity-4 I'm not sure that the BLI_delete_recycle is a good idea, since the Linux version does not recycle. I see that there are use cases where blender wants to delete without moving to the recycling bin, however the name is not suited for the Linux delete function

@Metricity-4 I'm not sure that the `BLI_delete_recycle` is a good idea, since the Linux version does not recycle. I see that there are use cases where blender wants to delete without moving to the recycling bin, however the name is not suited for the Linux delete function

Robert Guetzkow commented

2019-03-25 11:31:49 +01:00

Perhaps we could do the following: We could implement BLI_delete_recycle, but use #ifdef WIN32 in file_delete_exec to call this function only if it's Windows. This would avoid confusion about the Linux function call, which in your patch is just a wrapper for the regular BLI_delete.

Perhaps we could do the following: We could implement `BLI_delete_recycle`, but use `#ifdef WIN32` in `file_delete_exec` to call this function only if it's Windows. This would avoid confusion about the Linux function call, which in your patch is just a wrapper for the regular `BLI_delete`.

Robert Guetzkow commented

2019-03-25 12:49:19 +01:00

I've got the patch done, doing some last tests and then I'll submit a diff based on the code of @Metricity-4

Robert Guetzkow commented

2019-03-25 15:18:32 +01:00

@Metricity-4 thanks for implementing this! I hope you don't mind my improvement suggestions.

Kris commented

2019-03-25 22:28:03 +01:00

I was expecting feedback as this was my first time working with the blender codebase. The intention behind the separate function was if it could be used in other places and I wondered if linux or mac should have an implementation. It may also be worth reviewing if the regular delete functionality handles long paths too.

Robert Guetzkow commented

2019-03-25 22:50:13 +01:00

@Metricity-4 I agree, the separate function makes sense. It's a good idea to have this functionality for Linux and MacOS, too. I'm not sure if Linux has a unified way of handling the trash directory. If Linux and MacOS have the path set in an environment variable it should be a simple mv operation.

It may also be worth reviewing if the regular delete functionality handles long paths too.

Yes it can handle long paths. See the 'tip' box. The difference between DeleteFileA and DeleteFileW is that the former uses ANSI characters, the latter Unicode/UTF-16.

Sorry for hijacking your first contribution. I just thought this was something I could contribute to as well. Please don't feel discourage if the devs choose to not include your implementation using the legacy API. There is nothing wrong your implementation. If I would've known that we wouldn't need both the legacy and the new API, I would've left this for you to implement.

@Metricity-4 I agree, the separate function makes sense. It's a good idea to have this functionality for Linux and MacOS, too. I'm not sure if Linux has a unified way of handling the trash directory. If Linux and MacOS have the path set in an environment variable it should be a simple `mv` operation. > It may also be worth reviewing if the regular delete functionality handles long paths too. [Yes it can handle long paths. See the 'tip' box. ](https://docs.microsoft.com/en-us/windows/desktop/api/fileapi/nf-fileapi-deletefilea) The difference between `DeleteFileA` and `DeleteFileW` is that the former uses ANSI characters, the latter Unicode/UTF-16. Sorry for hijacking your first contribution. I just thought this was something I could contribute to as well. Please don't feel discourage if the devs choose to not include your implementation using the legacy API. There is nothing wrong your implementation. If I would've known that we wouldn't need both the legacy and the new API, I would've left this for you to implement.

Robert Guetzkow commented

2019-03-25 23:09:04 +01:00

@WilliamReynish I guess this should be addressed for Linux and MacOS as well. Currently remove() and rmdir() are used for Linux and MacOS which are equally destructive.

@WilliamReynish I guess this should be addressed for Linux and MacOS as well. Currently `remove()` and `rmdir()` are used for Linux and MacOS which are equally destructive.

William Reynish commented

2019-03-26 00:10:45 +01:00

@rjg Yes, absolutely. This should be done for all the OS’s.

Brecht Van Lommel commented

2019-03-26 00:54:31 +01:00

Added subscriber: @brecht

Brecht Van Lommel commented

2019-03-26 00:54:31 +01:00

Moving a file to the trash is actually quite a bit harder than mv:
https://specifications.freedesktop.org/trash-spec/trashspec-1.0.html

On Linux command line tools can be used, like this:
https://github.com/electron/electron/blob/master/atom/common/platform_util_linux.cc#L100

On macOS there are native API functions.

Moving a file to the trash is actually quite a bit harder than `mv`: https://specifications.freedesktop.org/trash-spec/trashspec-1.0.html On Linux command line tools can be used, like this: https://github.com/electron/electron/blob/master/atom/common/platform_util_linux.cc#L100 On macOS there are native API functions.

Ray Mairlot commented

2019-03-26 01:08:08 +01:00

Removed subscriber: @RayMairlot

Robert Guetzkow commented

2019-03-26 11:44:19 +01:00

@brecht You're right I was oversimplifying. What I meant was if $XDG_DATA_HOME is set we know where to move the files to (+handle metadata for restoring). The bigger challenge may be that some distributions don't follow the spec and $XDG_DATA_HOME isn't set. For instance on my Ubuntu VMs 16.04 and 18.04 both don't have $XDG_DATA_HOME set. Your linked code from Electron probably already solves this by using the cli tools, which will default to gio trash for Ubuntu with Gnome as far as I can tell.

@brecht You're right I was oversimplifying. What I meant was if `$XDG_DATA_HOME` is set we know where to move the files to (+handle metadata for restoring). The bigger challenge may be that some distributions don't follow the spec and `$XDG_DATA_HOME` isn't set. For instance on my Ubuntu VMs 16.04 and 18.04 both don't have `$XDG_DATA_HOME` set. Your linked code from Electron probably already solves this by using the cli tools, which will default to `gio trash` for Ubuntu with Gnome as far as I can tell.

William Reynish commented

2019-05-15 13:52:16 +02:00

Changed status from 'Open' to: 'Resolved'

William Reynish closed this issue

2019-05-15 13:52:16 +02:00

William Reynish self-assigned this 2019-05-15 13:52:16 +02:00

William Reynish commented

2019-05-15 13:52:16 +02:00

Marking as resolved for now. Now you cannot easily hard delete data at least.

Later we should add support for the system trash via native API's.

Marking as resolved for now. Now you cannot easily hard delete data at least. Later we should add support for the system trash via native API's.

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Blender file explorer hard deletes data #61412