fix: `ensureFile()` and `ensureFileSync()` for tight permissions

[egfx-notifications]

Implements the solution I proposed in #3339

Fixes #3339

[CLAassistant]

All committers have signed the CLA.

[kt3k]

Thanks for the fix. One thing I wonder about this solution is that this still prompt the user write permission for the parent dir. I wonder if reverting #3242 can be a better solution. If we revert #3242, ensureFile() even doesn't ask write permission for the parent dir when it already exists.

[egfx-notifications]

I wonder if reverting #3242 can be a better solution. If we revert #3242, ensureFile() even doesn't ask write permission for the parent dir when it already exists.

Yes, but then we would also reintroduce the race condition fixed by #3242

One thing I wonder about this solution is that this still prompt the user write permission for the parent dir.

You are right, I did not see this as the deno test runner uses --no-prompt implicitly and in my production code I also use --no-prompt explicitly. I just pushed another commit to only prompt after verifying that we do not have the permissions yet and the target directory does not exist. The only thing that bothers me is that I had to augment the function signature to avoid infinite recursion. If you can think of a better way to do this, I'd be happy to see it. My first approach with Deno.permissions.request() did not work, unfortunately, as it would prompt even if --no-prompt is specified, so I resorted to recursion and letting mkdir() do the prompting.

The current approach behaves like this:

• succeed if directory exists regardless of write permissions
• prompt for write permission if directory does not exist and prompting is allowed
• fail with PermissionDenied error from mkdir() if prompting is not allowed or permission was explicitly denied

@lucacasonato maybe you can have a look at this, too? Since the original fix #3242 concerns a bug that you reported

This being said, the ensureFile() and ensureFileSync() functions also do lstat() then writeFile(), so should probably be refactored as well.

[kt3k]

@egfx-notifications Thanks for updating, but the introduction of a new option looks too much to me. I'd suggest we would land with the previous version. Could you revert the last commit?

[kt3k]

I came up with another way to solve this issue. If we call lstat first, then mkdir, then catch only AlreadyExists, this solves both race condition and unnecessary write perm requirement without introducing new option. I'll try that in another branch.

[iuioiua]

I'm closing this in favour of #4041. Thank you for this, either way, @egfx-notifications.