Skip to content

[Xamarin.Android.Build.Tasks] Retry RemoveDirFixed on ERROR_DIR_NOT_EMPTY #11195

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

Merged
jonathanpeppers merged 1 commit into from
Apr 23, 2026
Merged

[Xamarin.Android.Build.Tasks] Retry RemoveDirFixed on ERROR_DIR_NOT_EMPTY #11195

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 10 additions & 3 deletions src/Xamarin.Android.Build.Tasks/Tasks/RemoveDirFixed.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,15 @@ public class RemoveDirFixed : AndroidTask
{
public override string TaskPrefix => "RDF";

const int ERROR_ACCESS_DENIED = -2147024891;
const int ERROR_SHARING_VIOLATION = -2147024864;
const int ERROR_ACCESS_DENIED = -2147024891; // 0x80070005
const int ERROR_SHARING_VIOLATION = -2147024864; // 0x80070020
// On Unix, .NET maps `ENOTEMPTY` from `rmdir(2)` to this Win32 HResult,
// so the same constant covers both Windows and Unix sources of
// "Directory not empty". This is observed on NTFS volumes mounted via
// the Linux `ntfs3` driver, where directory metadata can momentarily
// report children that have just been unlinked, but is not specific
// to that filesystem.
const int ERROR_DIR_NOT_EMPTY = -2147024751; // 0x80070091
Copy link
Copy Markdown
Member

@jonathanpeppers jonathanpeppers Apr 23, 2026

Choose a reason for hiding this comment

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

This is a real code listed here:


public override bool RunTask ()
{
Expand Down Expand Up @@ -80,7 +87,7 @@ public override bool RunTask ()
case UnauthorizedAccessException:
case IOException:
int code = Marshal.GetHRForException(e);
if ((code != ERROR_ACCESS_DENIED && code != ERROR_SHARING_VIOLATION) || retryCount >= attempts) {
if ((code != ERROR_ACCESS_DENIED && code != ERROR_SHARING_VIOLATION && code != ERROR_DIR_NOT_EMPTY) || retryCount >= attempts) {
Copy link
Copy Markdown

@github-actions github-actions Bot Apr 23, 2026

Choose a reason for hiding this comment

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

🤖 💡 Code organization — The negated-check pattern (code != A && code != B && code != C) is getting harder to scan as more error codes are added. Consider inverting to a positive check:

bool isRetriable = code is ERROR_ACCESS_DENIED or ERROR_SHARING_VIOLATION or ERROR_DIR_NOT_EMPTY;
if (!isRetriable || retryCount >= attempts) {
	throw;
}

This reads as "retry if retriable and retries remain" which matches the intent more naturally, and is easier to extend if a fourth HResult ever surfaces.

Rule: Reduce indentation / improve readability (Postmortem #62)

Copy link
Copy Markdown
Member

@jonathanpeppers jonathanpeppers Apr 23, 2026

Choose a reason for hiding this comment

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

I think it's ok.

throw;
};
break;
Expand Down
Loading