patchSet errno to EEXIST if mkdir returns EACCES or EPERM

... and the directory already exists. I.E. Cygwin is such a case: mkdir fail for mounts which reference directly to windows mounts ("drives").

---

The discussion, which ended up with this patch can be read here: http://www.cygwin.com/ml/cygwin/2006-01/msg01276.html

BTW, there is this: http://www.cygwin.com/ml/cygwin/2006-01/msg01380.html So this patch will probably be not needed soon.

Still I post it just for reference, or in case someone can't afford an upgrade.

 Makefile          |    8 ++++++++
 checkout-index.c  |    2 +-
 compat/mkdir.c    |   23 +++++++++++++++++++++++
 git-compat-util.h |    6 ++++++
 4 files changed, 38 insertions(+), 1 deletions(-)
 create mode 100644 compat/mkdir.c

c9f480f5d1e777cc0f4861e94c2449bd8c7cf73a

Alex Riesen <raa.lkml@gmail.com> writes:

Thanks for all the background information. Although it is very tempting for me to adopt your patch as an easy way out, I would feel *dirty* if I did so.

Eric Blake is right in his argument in that thread. Our code should not depend on the Linux EEXIST behaviour. The reason Cygwin folks want to be as close to Linux is to work around application bugs like this. Which is a valid concern for them, but it does not mean the application has license to depend on Linux behaviour.

We, as an application, should take a different attitude -- we should fix things to make things portable, not work things around, if both are equally easily doable. I do not object to using a function that has a semantics Linux mkdir() gives us, but calling that mkdir() does not feel quite right.

Also the wrapper implies that in our future use of mkdir() we cannot tell the difference between EEXIST and other errors if we later wanted to.

So let's look at our existing uses first:

apply.c:1577: if (mkdir(buf, 0777) < 0) { apply.c-1578- if (errno != EEXIST) apply.c-1579- break; apply.c-1580- } apply.c-1581- }

This is "we want to see buf directory exist and we would create one if there isn't". Instead of checking errno, we could stat as your patch does.

entry.c:15: if (mkdir(buf, 0777)) { entry.c-16- if (errno == EEXIST) { entry.c-17- struct stat st; entry.c:18: if (len > state->base_dir_len && state->force && !unlink(buf) && !mkdir(buf, 0777)) entry.c-19- continue; entry.c-20- if (!stat(buf, &st) && S_ISDIR(st.st_mode)) entry.c-21- continue; /* ok */ entry.c-22- }

This is a bit more involved. "we want to have buf directory and we would create it, and if force is given to have something under that directory, we would even unlink the nondirectory that sits there".

I asked "git-grep" where else we use mkdir(), and all other users want the semantics of the apply.c use quoted above.

So I'd rather see us create a generic helper function like this:

	int make_directory(const char *path, int force)
        {
		struct stat st;
		int mkdir_errno;
		if (!mkdir(path, 0777))
			return 0;
                mkdir_errno = errno;
		if (!lstat(path, &st) && S_ISDIR(st.st_mode))
                	return 0;
                if (!force) {
		bad:
                	errno = mkdir_errno;
                        return -1;
                }
                if (!unlink(path, &st) && !mkdir(path, 0777))
                	return 0;
                /* we might have failed to unlink an existing symlink
		 * which happens to point at an existing directory; that
                 * directory is not what we want here.
                 */
		if (!lstat(path, &st) && S_ISDIR(st.st_mode))
                	return 0;
		goto bad;
        }

and have current callers of mkdir() use it, regardless of the platform. It may not worth saving mkdir_errno, though.

Then everybody but entry.c one would say force=0, and entry.c one passes force appropriately using the condition it uses in its current if() statement.

Junio C Hamano, Mon, Jan 30, 2006 21:33:51 +0100:

errno may be worth saving. I find the process of finding
"why-the-f$%^-did-the-windows-broke-again" really tedious:
1. find the application which failed (grep for the die message)
2. put "%s", strerror(errno) in the die
3. retest
4. find out errno is 0 (success)
5. remove close(fd), munmap, whatever before the die
6. retest
7. repeat

I like it :)