STL icon indicating copy to clipboard operation
STL copied to clipboard

<filesystem>: create_directory vs create_directories return codes

Open DimRochette opened this issue 5 years ago • 11 comments

When you have path with trailing backslash the return codes of std::filesystem::create_directories and std::filesystem::create_directory for the same path are different. My problem is that return code are

  • different on msvc between <experimental/filesystem> and <filesystem>
  • different between msvc, clang and gcc

is someone right or is it implementation defined ?

#define _SILENCE_EXPERIMENTAL_FILESYSTEM_DEPRECATION_WARNING
#include <experimental/filesystem>
#include <filesystem>
#include <iostream>

int main()
{
	using std::cout;
	using std::endl;

	std::filesystem::remove("test1\\");
	std::filesystem::remove("test2\\");
	std::filesystem::remove("test3\\");
	std::filesystem::remove("test4\\");
	cout << "std::filesystem" <<endl;
	cout <<  std::filesystem::create_directory("test1\\")<< "\t:create_directory" << endl;
	cout <<  std::filesystem::create_directories("test2\\") << "\t:create_directories" << endl;
	cout << "std::experimental::filesystem" << endl;
	cout <<  std::experimental::filesystem::create_directory("test3\\") << "\t:create_directory" << endl;
	cout <<  std::experimental::filesystem::create_directories("test4\\") << "\t:create_directories" << endl;
}

this code will output

std::filesystem
1       :create_directory
0       :create_directories
std::experimental::filesystem
1       :create_directory
1       :create_directories

code to check on other compilers wandbox link

clang output for test/

std::filesystem
1	:create_directory
0	:create_directories

gcc output for test/

std::filesystem
1	:create_directory
1	:create_directories

DimRochette avatar Apr 22 '20 13:04 DimRochette

The language in http://eel.is/c++draft/fs.op.create.directories#1 says that it calls create_directory for each element of the path that doesn't exist: if create_directory succeeds by definition create_directories should too.

BillyONeal avatar Apr 22 '20 21:04 BillyONeal

Oh wait this isn't about whether it succeeds, it's whether the last create_directory call returns true or false. In that case I think msvc++'s and libc++'s current behavior are the correct behavior (libstdc++ and msvc experimental/filesystem) are wrong because create_directory returns whether a directory is created, and when creating the last path element, there is no directory created.

This smells like an LWG issue though.

BillyONeal avatar Apr 22 '20 21:04 BillyONeal

Check my math:

create_directory(p) "Creates the directory p resolves to" per [fs.op.create.directory]/1. On my machine, Either of create_directory("meow") or create_directory("meow\\") returns true and leaves a directory named "meow" in my current working directory. This implies that both pathnames "meow" and "meow\" resolve to the same file (or there's a bug in create_directory).

If "meow" and "meow\" resolve to the same file, then create_directories("meow\\") should not evaluate create_directory("meow\\") after create_directory("meow"), since the file to which the pathname "meow\" resolves already exists.

CaseyCarter avatar Apr 22 '20 22:04 CaseyCarter

I ran into this and the current result is unexpected. There is no semantic difference between "\mydir\" and "\mydir". However, the return result from create_directories is different. In the former, it is false and in the latter it is true. This means extra code for me to write to condition paths before calling create_directories.

shaunm-msft avatar Apr 16 '21 18:04 shaunm-msft

Check my math:

create_directory(p) "Creates the directory p resolves to" per [fs.op.create.directory]/1. On my machine, Either of create_directory("meow") or create_directory("meow\\") returns true and leaves a directory named "meow" in my current working directory. This implies that both pathnames "meow" and "meow" resolve to the same file (or there's a bug in create_directory).

If "meow" and "meow" resolve to the same file, then create_directories("meow\\") should not evaluate create_directory("meow\\") after create_directory("meow"), since the file to which the pathname "meow" resolves already exists.

Win32 normalization is biting us in the butt here; they do not resolve to the same file in general but GetFileAttributes ignores the last spurious .

I ran into this and the current result is unexpected. There is no semantic difference between "\mydir" and "\mydir". However, the return result from create_directories is different. In the former, it is false and in the latter it is true. This means extra code for me to write to condition paths before calling create_directories.

You're talking about leading slashes here which seems like a completely different bug.

BillyONeal avatar Apr 16 '21 18:04 BillyONeal

I'm talking about the trailing slash. For some reason they don't display in the comment I wrote. They are in there when I click Edit, but they don't display after the comment is saved. Maybe I need to add another slash, but then that's two slashes and I'm not talking about that issue: Windows is permissive about multiple consecutive slashes, which is a completely different issue from what we're talking about here.

I am running this on Windows, and Windows semantics may be different from Linux with regard to the treatment of trailing slashes. I haven't tried it on Linux, so I can't speak to the differences.

The current create_directories implementation loops by slash and still goes through the motions for the trailing slash resulting in a change in return value even though it is a noop. So the return value when there is a trailing slash is always false.

While there might be a great technical argument about trailing slash differences and whether it alters the meaning of what p resolves to, always returning false if there's a trailing slash causes a consumption issue.

To deal with this, the consumer must check for a trailing slash and remove it before calling create_directories in order to know when a directory is create and when it isn't -- which is the purpose of the return value to begin with.

I'd like to argue that the language should support reasonable consumption. And that a trailing slash shouldn't result in the function always returning false. That's just begging for a different library that's more permissive to intuitive usage. Since this library isn't set in stone yet, we should adjust it while we still can so people don't need a competing library.

shaunm-msft avatar Apr 16 '21 19:04 shaunm-msft

I've updated your comment to display the trailing backslash (by using backticks for code font).

StephanTLavavej avatar Apr 29 '21 03:04 StephanTLavavej

Also reported as DevCom-10490142.

StephanTLavavej avatar Oct 24 '23 22:10 StephanTLavavej

Simpler example, which I coded up to report as a bug, before discovering that it had been reported before:

#include <cstdio>
#include <filesystem>

int main( int, char*[] )
{
	#define DIR "foo/"

	printf( "Directory \"" DIR "\" %s.\n",
		std::filesystem::exists(DIR) ? "exists" : "does not exist"
	);

	bool succeeded = std::filesystem::create_directories(DIR);
	printf( "Creating \"" DIR "\" %s!\n", succeeded?"succeeded":"failed" );

	printf( "Directory \"" DIR "\" %s.\n",
		std::filesystem::exists(DIR) ? "exists" : "does not exist"
	);

	return 0;
}

Typical output is:

Directory "foo/" does not exist.
Creating "foo/" failed!
Directory "foo/" exists.

This is pretty obviously extremely incorrect.

Now, the standard seems pretty clear to me on what should happen (as a commenter above cited, create_directories(⋯) "Returns: true if a new directory was created for the directory p resolves to, otherwise false."), so basically if "foo/" resolves to "foo", then it should return true—which it indeed does on other platforms.

However, with either interpretation, the above example demonstrates an extant bug. If "foo/" resolves to "foo", then create_directories(⋯) is not respecting that, and is broken. On the other hand, if "foo/" is not supposed to resolve to "foo", then e.g. exists(⋯) is broken. (That said, I cannot imagine a sane and loving universe where "foo/" mustn't resolve to "foo" . . .)

geometrian avatar Mar 26 '24 02:03 geometrian

(That said, I cannot imagine a sane and loving universe where "foo/" mustn't resolve to "foo" . . .)

Welcome to symlinks world, our world

BillyONeal avatar Mar 26 '24 23:03 BillyONeal

bool succeeded = std::filesystem::create_directories(DIR);

It should be noted, the return value of create_directories is not intended to be whether it 'succeeded'. Returning false because the directory already exists is success.

BillyONeal avatar Mar 26 '24 23:03 BillyONeal