Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix deleting of directories on Linux #58335

Merged
merged 1 commit into from
Mar 10, 2022

Conversation

maiself
Copy link
Contributor

@maiself maiself commented Feb 20, 2022

Trailing slash of directories was mishandled, and incorrect derived paths
were formed. Stripping the slash fixes this.

@akien-mga
Copy link
Member

Some of the style changes are not valid for our clang-format configuration: https://github.com/godotengine/godot/runs/5261982845?check_suite_focus=true

(It might work if you keep the operator before the line break, and use two tabs for continuation)

@akien-mga
Copy link
Member

Do you have an example of how the bug can be reproduced exactly?

@maiself
Copy link
Contributor Author

maiself commented Feb 20, 2022

Thanks for the reply, I'll give your suggestion on formatting a try, and see if I can get format checks going locally.

Do you have an example of how the bug can be reproduced exactly?

I recently did some moving around and bind mounting of my ~/src and godot repository checkouts for faster file access on an SSD. The configuration caused this function to fail pretty far down: gio won't trash across mount points, kioclient5 and gvfs-trash not available, $mnt_point/.Trash-1000 didn't exist.

$XDG_DATA_HOME/Trash and $HOME/.local/share/Trash are good tho, which brings us down to:

// Rename our resource before moving it to the trash can.

and this expression (which is the most important thing replaced by this patch):

p_path.get_base_dir() + "/" + file_name

Step by step...

p_path = "/path/dir/" (Thing to delete, passed in as argument)
file_name = "dir" (Future name of thing in trash, from a few lines up file_name = p_path.get_file(); if (...)

substitute: "/path/dir/".get_base_dir() + "/" + "dir"
eval: "/path/dir" + "/" + "dir"
concat: "/path/dir/dir" (This is not a vaild path)

Because this expression produces an invalid path and is used for all subsequent operations, nothing else will succeed.

Trailing slash of directories was mishandled, and incorrect derived paths
were formed. Stripping the slash fixes this.
@maiself maiself force-pushed the fix-directory-delete-linux branch from 9ac00cd to dfdc1c1 Compare February 20, 2022 13:39
@maiself
Copy link
Contributor Author

maiself commented Feb 20, 2022

I wasn't able to figure out how to make clang-format happy with breaking up the long lines, it accepted one line with your suggestion, but continued rejecting the others.

@akien-mga akien-mga merged commit 7866c98 into godotengine:master Mar 10, 2022
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants