bug#41001: mkdir: cannot create directory ‘test’: File exists

2020-05-01 Thread Paul Eggert
On 5/1/20 1:21 PM, Jonny Grant wrote:
> yes, the fix pretty trivial for mkdir as you highlight EISDIR:
> stat(), S_ISDIR(sb.st_mode), and set errno to EISDIR or output 
> strerror(EISDIR)

That would introduce a race condition, and wouldn't behave correctly if some
other process changes the destination from a regular file to a directory between
the time we call mkdir and the time that we call stat.





bug#41001: mkdir: cannot create directory ‘test’: File exists

2020-05-01 Thread Jonny Grant




On 01/05/2020 19:07, Paul Eggert wrote:

On 5/1/20 9:16 AM, Jonny Grant wrote:

rm: cannot remove 'test': Is a directory


That's because rm used unlink which failed with EISDIR, which is a different
error number.


yes, the fix pretty trivial for mkdir as you highlight EISDIR:
stat(), S_ISDIR(sb.st_mode), and set errno to EISDIR or output 
strerror(EISDIR)


$ mkdir test
mkdir: cannot create directory ‘test’: Is a directory


Consider this example:

 $ >d # Create an empty regular file.
 $ mkdir d
 mkdir: cannot create directory ‘d’: File exists

Here the system call mkdir("d", 0777) failed with errno == EEXIST (File exists).
Presumably you wouldn't object to the diagnostic here because d is a regular
file, not a directory. But the mkdir system call fails in exactly the same way
if d is a directory, so the error message is the same in both cases.


Exactly, UNIX didn't create separate errno for files and directories, 
it's the same limitation with ENOENT. As a developer, we handle it 
ourselves, as it's easy enough to call stat() like other package 
maintainers do, as you can see in binutils.



Directories are files, so the error message is correct even if it confused you.
I don't see any portable and efficient way to make the diagnostic less confusing
for you, without also making diagnostic incorrect in some other scenarios (such
as the scenario described above).


Feels like the fix I already proposed does not have any incorrect impact 
in the other scenario you describe? Do correct me if I am missing something.


Yes, as a developer I know everything is actually a file, but users 
don't. Users will call it a folder, or a directory. I didn't go over 
UNIX everything-is-a-file in my bug report because everyone here knows 
already.


This one is an simple fix, but it's clear no one wants to introduce the 
change, no worries.


Cheers, Jonny





bug#41001: mkdir: cannot create directory ‘test’: File exists

2020-05-01 Thread Paul Eggert
On 5/1/20 9:16 AM, Jonny Grant wrote:
> rm: cannot remove 'test': Is a directory

That's because rm used unlink which failed with EISDIR, which is a different
error number.

Consider this example:

$ >d # Create an empty regular file.
$ mkdir d
mkdir: cannot create directory ‘d’: File exists

Here the system call mkdir("d", 0777) failed with errno == EEXIST (File exists).
Presumably you wouldn't object to the diagnostic here because d is a regular
file, not a directory. But the mkdir system call fails in exactly the same way
if d is a directory, so the error message is the same in both cases.

Directories are files, so the error message is correct even if it confused you.
I don't see any portable and efficient way to make the diagnostic less confusing
for you, without also making diagnostic incorrect in some other scenarios (such
as the scenario described above).





bug#41001: mkdir: cannot create directory ‘test’: File exists

2020-05-01 Thread Eric Blake

tag 41001 notabug
thanks

On 5/1/20 10:06 AM, Jonny Grant wrote:

Hello!

Can this error message be clarified? The directory already exists, it is 
not a file.


By one definition, a directory _is_ a file, just with different 
semantics (in the same way a block device, character device, symlink, 
fifo, or socket can also be a file).  It is not a regular file, but "a 
file" is any entry stored in a directory, and as subdirectories are 
stored in a directory, they count as files.




lib/mkdir-p.c:200 contains this line of code that triggers below:-

error (0, mkdir_errno, _("cannot create directory %s"), quote (dir));


The error message in question is coming from libc's strerror() function; 
if you want a different error message for EEXIST, you'll have to 
convince glibc to update their error string tables.  It is not something 
that coreutils directly controls.  And while we could indeed output a 
custom message instead of using strerror(), that would confuse people 
who have grown used to the strerror() message.




As it's easy enough to know that the reason mkdir fails is because 
'test' a directory that already exists.


Easy enough to check with stat() and S_ISDIR(sb.st_mode)

Can this be changed? Maybe I can make a patch for it.
Jonny



$ mkdir test
$ mkdir test
mkdir: cannot create directory ‘test’: File exists


If nothing else, you may want to consider using 'mkdir -p test', which 
specifically checks if the reason for an EEXIST failure is because the 
directory already exists.  Once you do that, you don't need to patch 
coreutils.


Thus, I'm closing this as not a bug; but feel free to respond further 
with any more comments on the topic.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org






bug#41001: mkdir: cannot create directory ‘test’: File exists

2020-05-01 Thread Jonny Grant

Hello!

Can this error message be clarified? The directory already exists, it is 
not a file.


lib/mkdir-p.c:200 contains this line of code that triggers below:-

error (0, mkdir_errno, _("cannot create directory %s"), quote (dir));

As it's easy enough to know that the reason mkdir fails is because 
'test' a directory that already exists.


Easy enough to check with stat() and S_ISDIR(sb.st_mode)

Can this be changed? Maybe I can make a patch for it.
Jonny



$ mkdir test
$ mkdir test
mkdir: cannot create directory ‘test’: File exists
$ mkdir --version
mkdir (GNU coreutils) 8.28
Copyright (C) 2017 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later 
.

This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Written by David MacKenzie.
$