Skip to content

Ticket #4720: delete resource forks on macOS last #4722

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.

Sign up for GitHub

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Ticket #4720: delete resource forks on macOS last #4722

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a009062
Ticket #4720: delete resource forks on macOS last to avoid recursive …
zyv Jun 9, 2025
6d1544c
fixup! Ticket #4720: delete resource forks on macOS last to avoid rec…
zyv Jun 9, 2025
e563773
fixup! fixup! Ticket #4720: delete resource forks on macOS last to av…
zyv Jun 9, 2025
e7aa5ec
fixup! fixup! fixup! Ticket #4720: delete resource forks on macOS las…
zyv Jun 10, 2025
f39514d
fixup! fixup! fixup! fixup! Ticket #4720: delete resource forks on ma…
zyv Jun 10, 2025
b2363ba
fixup! fixup! fixup! fixup! fixup! Ticket #4720: delete resource fork…
zyv Jun 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions lib/fs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,11 @@
#define DIR_IS_DOT(x) ((x)[0] == '.' && (x)[1] == '\0')
#define DIR_IS_DOTDOT(x) ((x)[0] == '.' && (x)[1] == '.' && (x)[2] == '\0')

/* On macOS, if a file system doesn't support resource forks, metadata is automatically stored in
* shadow files beginning with `._`. They are automatically managed by the OS and will reappear if
* deleted. */
#define FILE_IS_RESOURCE_FORK(x) ((x)[0] == '.' && (x)[1] == '_' && (x)[2] != '\0')

/*** enums ***************************************************************************************/

/*** structures declarations (and typedefs of structures)*****************************************/
Expand Down
35 changes: 34 additions & 1 deletion src/filemanager/file.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1439,9 +1439,18 @@ try_erase_dir (file_op_context_t *ctx, const vfs_path_t *vpath)
abort -> cancel stack
ignore -> warn every level, gets default
ignore_all -> remove as much as possible

This function should either be called with delete_resource_forks = TRUE always, or called twice,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this feels complex and dirty.
my first thought would be to just put files that look like forks into a list, and delete them in a second loop. it's kinda like your sorting idea, just more targeted.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about it, but this looks much more complicated to me. I'm not ready to implement anything like this in the current code base.

I thought the functions would retrieve a list of items to delete from the panels. However, it's a recursive call tree connected with callbacks to the progress dialog. The easiest and safest method seems to be simply calling the recursion twice.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

easiest, i guess. but safest ...? this looks like an invitation for double accounting, dialogs asking twice for the same error, etc. it's much harder to guarantee that it works in every case, and that it keeps working.

i'm not sure why you think the separate loop would be much more complicated. it would be a little bit more code (part of the loop body would have to be duplicated or factored out), but seems rather straight-forward.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's complicated because it's difficult for me to do. Maybe someone can help.

first with FALSE and then with TRUE to make sure that no errors pop up due to resource forks being
deleted on macOS and then being re-created on the fly by the OS.
*/
static FileProgressStatus
recursive_erase (file_op_context_t *ctx, const vfs_path_t *vpath)
recursive_erase (file_op_context_t *ctx, const vfs_path_t *vpath
#ifdef __APPLE__
,
const gboolean delete_resource_forks
#endif
)
{
struct vfs_dirent *next;
DIR *reading;
Expand All @@ -1466,10 +1475,26 @@ recursive_erase (file_op_context_t *ctx, const vfs_path_t *vpath)
vfs_path_free (tmp_vpath, TRUE);
return FILE_RETRY;
}
#ifdef __APPLE__
if (S_ISDIR (buf.st_mode))
{
return_status = recursive_erase (ctx, tmp_vpath, FALSE);
if (return_status != FILE_ABORT)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make this compiled conditionally. Users of OSes other than macOS must not feel bad.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it will put users of other operating systems at a disadvantage, but if you think this is the right approach, I can compile it conditionally. I was concerned that it would make the code more difficult to maintain.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i also had the thought to make this os-specific. but then, what if it's an nfs mount from a mac?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i also had the thought to make this os-specific. but then, what if it's an nfs mount from a mac?

Yes, this is a good argument.

But what about non-macOS users? What is the disadvantage for them? Only if there are files beginning with ._ in the tree will anything be different; in all other cases, there will be no change.

I think this is extremely rare on other systems, and when it does happen, the impact will be negligible, so I would rather drop my fixups; they don't look good.

return_status = recursive_erase (ctx, tmp_vpath, TRUE);
}
else
{
if (!delete_resource_forks && FILE_IS_RESOURCE_FORK (next->d_name))
return_status = FILE_SKIP;
else
return_status = erase_file (ctx, tmp_vpath);
}
#else
if (S_ISDIR (buf.st_mode))
return_status = recursive_erase (ctx, tmp_vpath);
else
return_status = erase_file (ctx, tmp_vpath);
#endif
vfs_path_free (tmp_vpath, TRUE);
}
mc_closedir (reading);
Expand Down Expand Up @@ -3351,7 +3376,15 @@ erase_dir (file_op_context_t *ctx, const vfs_path_t *vpath)
// not empty
error = query_recursive (ctx, vfs_path_as_str (vpath));
if (error == FILE_CONT)
{
#ifdef __APPLE__
error = recursive_erase (ctx, vpath, FALSE);
if (error != FILE_ABORT)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make this compiled conditionally.

error = recursive_erase (ctx, vpath, TRUE);
#else
error = recursive_erase (ctx, vpath);
#endif
}
return error;
}

Expand Down
Loading