Godot: Re-Export of Mac throws error

[rcorre]

Godot version:

3.2.2.stable.custom_build.bc8eea3e9

OS/device including version:

Linux 5.8.8-arch1-1

Issue description:

Running "Export All" multiple times can cause an error as Godot fails to overwrite the prior export.

Steps to reproduce:

1. Set up an export for Mac (Runnable) .zip
2. Run "Export All"
3. Run "Export All" again
4. See error:

platform/x11/os_x11.cpp:3438 - move_to_trash: Could not move the resource "/home/rcorre/.cache/godot/JetHook.app" to the trash can "/home/rcorre/.local/share/Trash/files"

Manually removing "/home/rcorre/.cache/godot/JetHook.app" avoids the error.

Minimal reproduction project:

[aaronfranke]

Please re-test with Godot 3.2.3 stable. Mac exporting is known to be broken in 3.2.2.

[akien-mga]

Please re-test with Godot 3.2.3 stable. Mac exporting is known to be broken in 3.2.2.

I don't think it would make a difference here, the issue is more with move_to_trash likely not handling folder deletion.

[bruvzg]

Linux move_to_trash seems to use mv to move folders to the trash, without renaming it. If the .app folder from the previous export already exists in the trashcan it fails.

[rcorre]

Could be related to https://github.com/godotengine/godot/issues/27526?

[bruvzg]

Could be related to #27526?

It's probably the same (at least move_to_trash part).

Bad solution: add -f to the mv and overwrite existing copy.
Better solution: check for existing copy and add number to newly deleted one (similar to node duplication naming).

[InfiniteAnts]

Can I work on this issue? Will I be working on the 3.2 branch or the master branch?

[rcorre]

@AnantAhuja can you check if this is an issue on master? I did not.
If it is, then you should fix on master and backport to 3.2.
Thanks!

[fredkozlowski]

Hi, I've just submitted a PR for this on 3.2 that appends a number to the exports moved to trash. I just saw that you mentioned it would be good to test on master branch. I'll test that soon and cancel my existing PR? Or should I cancel it now and test on master then resubmit?

[rcorre]

The pull-request template says:

Pull requests should always be made for the `master` branch first, as that's
where development happens and the source of all future stable release branches.

Relevant fixes are cherry-picked for stable branches as needed.

Do not create a pull request for stable branches unless the change is already
available in the `master` branch and it cannot be easily cherry-picked.
Alternatively, if the change is only relevant for that branch (e.g. rendering
fixes for the 3.2 branch).

I can't really say, as I'm not a maintainer, but it would be preferable to target master first (and verify that this issue actually exists on master and wasn't somehow fixed).

[fredkozlowski]

Gotcha, will check. Thanks for the help.

[rcorre]

I just hit this again, and I noticed it is moving a file from .cache. I'd assume anything in .cache is safe to delete, so can we just rm it instead of Trashing it?