[PATCH] Solaris 5.8 returns ENOTDIR for inappropriate renames.

[PATCH] Solaris 5.8 returns ENOTDIR for inappropriate renames.

[Jason Riedy]

The reflog code clears empty directories when rename returns
either EISDIR or ENOTDIR.  Seems to be the only place.

Signed-off-by: Jason Riedy <ejr@cs.berkeley.edu>
---
   Haven't been keeping up with the list, so I apologize if I've
   missed discussions related to these changes.

 refs.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/refs.c b/refs.c
index 689ac50..851b573 100644
--- a/refs.c
+++ b/refs.c
@@ -837,7 +837,7 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
 
  retry:
 	if (log && rename(git_path("tmp-renamed-log"), git_path("logs/%s", newref))) {
-		if (errno==EISDIR) {
+		if (errno==EISDIR || errno==ENOTDIR) {
 			if (remove_empty_directories(git_path("logs/%s", newref))) {
 				error("Directory not empty: logs/%s", newref);
 				goto rollback;
-- 
1.5.0.rc1.gf4b6c

Re: [PATCH] Solaris 5.8 returns ENOTDIR for inappropriate renames.

[Junio C Hamano]

Jason Riedy <ejr@EECS.Berkeley.EDU> writes:

> The reflog code clears empty directories when rename returns
> either EISDIR or ENOTDIR.  Seems to be the only place.
>
> Signed-off-by: Jason Riedy <ejr@cs.berkeley.edu>
> ---
>    Haven't been keeping up with the list, so I apologize if I've
>    missed discussions related to these changes.
>
>  refs.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 689ac50..851b573 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -837,7 +837,7 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
>  
>   retry:
>  	if (log && rename(git_path("tmp-renamed-log"), git_path("logs/%s", newref))) {
> -		if (errno==EISDIR) {
> +		if (errno==EISDIR || errno==ENOTDIR) {
>  			if (remove_empty_directories(git_path("logs/%s", newref))) {
>  				error("Directory not empty: logs/%s", newref);
>  				goto rollback;

Sorry, I do not understand.

Before this codepath, we have done safe_create_directories() and
should have already errored out if the parent directory of
"logs/$newref" couldn't have been created for whatever reason.
So when this rename fails, we have a file ".git/tmp-renamed-log"
and a directory ".git/logs/foo", and we are trying to rename the
former to ".git/logs/foo/bar".

Now ".git/logs/foo/bar" might already exist as a directory, and
this error path is attempting to catch EISDIR that comes out
from it (and in that case, if there is nothing but empty
directories in the hierarchy under .git/logs/foo/bar, we 
remove them and retry).

Does Solaris give ENOTDIR in such a case?

Re: [PATCH] Solaris 5.8 returns ENOTDIR for inappropriate renames.

[Jason Riedy]

And Junio C Hamano writes:
> > -		if (errno==EISDIR) {
> > +		if (errno==EISDIR || errno==ENOTDIR) {

> Now ".git/logs/foo/bar" might already exist as a directory, and
> this error path is attempting to catch EISDIR that comes out
> from it (and in that case, if there is nothing but empty
> directories in the hierarchy under .git/logs/foo/bar, we 
> remove them and retry).
> 
> Does Solaris give ENOTDIR in such a case?

Exactly.  rename(file, directory) produces an error.  Some
systems associate the error with the directory and give EISDIR.
Solaris (at least 5.8) associates the error with the file and
gives ENOTDIR.

Looks like SUS declares EISDIR as the correct errno.  I didn't
realize that...  I don't think it's worth a full compat wrapper
just for this, but I could be wrong.

It is worth including your very lucid explanation in the
routine's comments, though.

Jason