[Bug libstdc++/101510] std::filesystem::create_directory on an existing symlink to a directory
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101510 Jonathan Wakely changed: What|Removed |Added Target Milestone|--- |9.5 Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #24 from Jonathan Wakely --- Fixed for 9.5, 10.4 and 11.3, thanks for the report.
[Bug libstdc++/101510] std::filesystem::create_directory on an existing symlink to a directory
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101510 --- Comment #23 from CVS Commits --- The releases/gcc-9 branch has been updated by Jonathan Wakely : https://gcc.gnu.org/g:f8ac77533951d79d4e6a65841aa30293b2f11fdd commit r9-9669-gf8ac77533951d79d4e6a65841aa30293b2f11fdd Author: Jonathan Wakely Date: Tue Jul 20 18:15:48 2021 +0100 libstdc++: Fix create_directories to resolve symlinks [PR101510] When filesystem__create_directories checks to see if the path already exists and resolves to a directory, it uses filesystem::symlink_status, which means it reports an error if the path is a symlink. It should use filesystem::status, so that the target directory is detected, and no error is reported. Signed-off-by: Jonathan Wakely libstdc++-v3/ChangeLog: PR libstdc++/101510 * src/c++17/fs_ops.cc (fs::create_directories): Use status instead of symlink_status. * src/filesystem/ops.cc (fs::create_directories): Likewise. * testsuite/27_io/filesystem/operations/create_directories.cc: Check symlink to existing directory. * testsuite/27_io/filesystem/operations/create_directory.cc: Do not test with symlinks on Windows. * testsuite/experimental/filesystem/operations/create_directories.cc: Check symlink to existing directory. * testsuite/experimental/filesystem/operations/create_directory.cc: Do not test with symlinks on Windows. (cherry picked from commit 124eaa50e0a34f5f89572c1aa812c50979da58fc)
[Bug libstdc++/101510] std::filesystem::create_directory on an existing symlink to a directory
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101510 --- Comment #22 from CVS Commits --- The releases/gcc-9 branch has been updated by Jonathan Wakely : https://gcc.gnu.org/g:1a5cc427fbc796fd97218b3d9b80b6a22268893a commit r9-9668-g1a5cc427fbc796fd97218b3d9b80b6a22268893a Author: Jonathan Wakely Date: Tue Jul 20 12:35:37 2021 +0100 libstdc++: Add more tests for filesystem::create_directory [PR101510] Signed-off-by: Jonathan Wakely libstdc++-v3/ChangeLog: PR libstdc++/101510 * src/c++17/fs_ops.cc (create_dir): Adjust whitespace. * testsuite/27_io/filesystem/operations/create_directory.cc: Test creating directory with name of existing symlink to directory. * testsuite/experimental/filesystem/operations/create_directory.cc: Likewise. (cherry picked from commit 0c4ae4ff46b1d7633f1e06f57d348b5817b8f640)
[Bug libstdc++/101510] std::filesystem::create_directory on an existing symlink to a directory
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101510 --- Comment #21 from CVS Commits --- The releases/gcc-10 branch has been updated by Jonathan Wakely : https://gcc.gnu.org/g:86bdcd21a1be008df28648b185c12221d917a166 commit r10-10031-g86bdcd21a1be008df28648b185c12221d917a166 Author: Jonathan Wakely Date: Tue Jul 20 18:15:48 2021 +0100 libstdc++: Fix create_directories to resolve symlinks [PR101510] When filesystem__create_directories checks to see if the path already exists and resolves to a directory, it uses filesystem::symlink_status, which means it reports an error if the path is a symlink. It should use filesystem::status, so that the target directory is detected, and no error is reported. Signed-off-by: Jonathan Wakely libstdc++-v3/ChangeLog: PR libstdc++/101510 * src/c++17/fs_ops.cc (fs::create_directories): Use status instead of symlink_status. * src/filesystem/ops.cc (fs::create_directories): Likewise. * testsuite/27_io/filesystem/operations/create_directories.cc: Check symlink to existing directory. * testsuite/27_io/filesystem/operations/create_directory.cc: Do not test with symlinks on Windows. * testsuite/experimental/filesystem/operations/create_directories.cc: Check symlink to existing directory. * testsuite/experimental/filesystem/operations/create_directory.cc: Do not test with symlinks on Windows. (cherry picked from commit 124eaa50e0a34f5f89572c1aa812c50979da58fc)
[Bug libstdc++/101510] std::filesystem::create_directory on an existing symlink to a directory
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101510 --- Comment #20 from CVS Commits --- The releases/gcc-10 branch has been updated by Jonathan Wakely : https://gcc.gnu.org/g:f28d0e3a39246767b5fcdbe5c2bc005a45253246 commit r10-10030-gf28d0e3a39246767b5fcdbe5c2bc005a45253246 Author: Jonathan Wakely Date: Tue Jul 20 12:35:37 2021 +0100 libstdc++: Add more tests for filesystem::create_directory [PR101510] Signed-off-by: Jonathan Wakely libstdc++-v3/ChangeLog: PR libstdc++/101510 * src/c++17/fs_ops.cc (create_dir): Adjust whitespace. * testsuite/27_io/filesystem/operations/create_directory.cc: Test creating directory with name of existing symlink to directory. * testsuite/experimental/filesystem/operations/create_directory.cc: Likewise. (cherry picked from commit 0c4ae4ff46b1d7633f1e06f57d348b5817b8f640)
[Bug libstdc++/101510] std::filesystem::create_directory on an existing symlink to a directory
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101510 --- Comment #19 from CVS Commits --- The releases/gcc-11 branch has been updated by Jonathan Wakely : https://gcc.gnu.org/g:bde28c60c70079416dd2181f882c0694e019eaef commit r11-8850-gbde28c60c70079416dd2181f882c0694e019eaef Author: Jonathan Wakely Date: Tue Jul 20 18:15:48 2021 +0100 libstdc++: Fix create_directories to resolve symlinks [PR101510] When filesystem__create_directories checks to see if the path already exists and resolves to a directory, it uses filesystem::symlink_status, which means it reports an error if the path is a symlink. It should use filesystem::status, so that the target directory is detected, and no error is reported. Signed-off-by: Jonathan Wakely libstdc++-v3/ChangeLog: PR libstdc++/101510 * src/c++17/fs_ops.cc (fs::create_directories): Use status instead of symlink_status. * src/filesystem/ops.cc (fs::create_directories): Likewise. * testsuite/27_io/filesystem/operations/create_directories.cc: Check symlink to existing directory. * testsuite/27_io/filesystem/operations/create_directory.cc: Do not test with symlinks on Windows. * testsuite/experimental/filesystem/operations/create_directories.cc: Check symlink to existing directory. * testsuite/experimental/filesystem/operations/create_directory.cc: Do not test with symlinks on Windows. (cherry picked from commit 124eaa50e0a34f5f89572c1aa812c50979da58fc)
[Bug libstdc++/101510] std::filesystem::create_directory on an existing symlink to a directory
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101510 --- Comment #18 from CVS Commits --- The releases/gcc-11 branch has been updated by Jonathan Wakely : https://gcc.gnu.org/g:c5f17274aabe61bb0193b8b68283c1f1da5ee699 commit r11-8849-gc5f17274aabe61bb0193b8b68283c1f1da5ee699 Author: Jonathan Wakely Date: Tue Jul 20 12:35:37 2021 +0100 libstdc++: Add more tests for filesystem::create_directory [PR101510] Signed-off-by: Jonathan Wakely libstdc++-v3/ChangeLog: PR libstdc++/101510 * src/c++17/fs_ops.cc (create_dir): Adjust whitespace. * testsuite/27_io/filesystem/operations/create_directory.cc: Test creating directory with name of existing symlink to directory. * testsuite/experimental/filesystem/operations/create_directory.cc: Likewise. (cherry picked from commit 0c4ae4ff46b1d7633f1e06f57d348b5817b8f640)
[Bug libstdc++/101510] std::filesystem::create_directory on an existing symlink to a directory
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101510 --- Comment #17 from Madhu --- * "redi at gcc dot gnu.org" Wrote on Tue, 20 Jul 2021 19:38:18 + > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101510 > Fixed on trunk now, I'll backport it too, but not in time for 11.2 Thanks!
[Bug libstdc++/101510] std::filesystem::create_directory on an existing symlink to a directory
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101510 --- Comment #16 from Jonathan Wakely --- Fixed on trunk now, I'll backport it too, but not in time for 11.2
[Bug libstdc++/101510] std::filesystem::create_directory on an existing symlink to a directory
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101510 --- Comment #15 from CVS Commits --- The master branch has been updated by Jonathan Wakely : https://gcc.gnu.org/g:124eaa50e0a34f5f89572c1aa812c50979da58fc commit r12-2421-g124eaa50e0a34f5f89572c1aa812c50979da58fc Author: Jonathan Wakely Date: Tue Jul 20 18:15:48 2021 +0100 libstdc++: Fix create_directories to resolve symlinks [PR101510] When filesystem__create_directories checks to see if the path already exists and resovles to a directory, it uses filesystem::symlink_status, which means it reports an error if the path is a symlink. It should use filesystem::status, so that the target directory is detected, and no error is reported. Signed-off-by: Jonathan Wakely libstdc++-v3/ChangeLog: PR libstdc++/101510 * src/c++17/fs_ops.cc (fs::create_directories): Use status instead of symlink_status. * src/filesystem/ops.cc (fs::create_directories): Likewise. * testsuite/27_io/filesystem/operations/create_directories.cc: * testsuite/27_io/filesystem/operations/create_directory.cc: Do not test with symlinks on Windows. * testsuite/experimental/filesystem/operations/create_directories.cc: * testsuite/experimental/filesystem/operations/create_directory.cc: Do not test with symlinks on Windows.
[Bug libstdc++/101510] std::filesystem::create_directory on an existing symlink to a directory
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101510 --- Comment #14 from Madhu --- * "redi at gcc dot gnu.org" Wrote on Tue, 20 Jul 2021 16:59:20 + > Well your "initial problem" was talking exclusively about > create_directory, which works perfectly. > > There is a problem is a **different** function, however when you > report a bug saying "A doesn't work" it's hard to know that you > meant "B doesn't work". > > This is why you're supposed to provide a testcase, so we don't have > to guess what you're actually trying to say. Mea maxima culpa. The title is wrong - create_directories was the only function I tested before posting.
[Bug libstdc++/101510] std::filesystem::create_directory on an existing symlink to a directory
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101510 --- Comment #13 from Jonathan Wakely --- (In reply to Madhu from comment #11) > I think my initial problem remains Well your "initial problem" was talking exclusively about create_directory, which works perfectly. There is a problem is a **different** function, however when you report a bug saying "A doesn't work" it's hard to know that you meant "B doesn't work". This is why you're supposed to provide a testcase, so we don't have to guess what you're actually trying to say.
[Bug libstdc++/101510] std::filesystem::create_directory on an existing symlink to a directory
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101510
Jonathan Wakely changed:
What|Removed |Added
Status|WAITING |ASSIGNED
--- Comment #12 from Jonathan Wakely ---
This is the code from create_directories:
file_status st = symlink_status(p, ec);
if (is_directory(st))
return false;
else if (ec && !status_known(st))
return false;
else if (exists(st))
{
if (!ec)
ec = std::make_error_code(std::errc::not_a_directory);
return false;
}
The first line should use status not symlink_status, that's the bug.
[Bug libstdc++/101510] std::filesystem::create_directory on an existing symlink to a directory
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101510 --- Comment #11 from Madhu --- sorry for the typos in my last message - i should've proofread - but they should be fairly obvious. ( I wrote "p" instead of p, and fff instead of xxx, etc.) I think my initial problem remains
[Bug libstdc++/101510] std::filesystem::create_directory on an existing symlink to a directory
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101510 Jonathan Wakely changed: What|Removed |Added Resolution|INVALID |--- Status|RESOLVED|WAITING --- Comment #10 from Jonathan Wakely --- So then that would be a problem with create_directories, not create_directory (which is the subject of the bug you reported). Could you please provide an actual (complete) testcase showing the actual problem?
[Bug libstdc++/101510] std::filesystem::create_directory on an existing symlink to a directory
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101510
--- Comment #9 from Madhu ---
* "redi at gcc dot gnu.org"
Wrote on Tue, 20 Jul 2021 16:25:44 +
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101510
>
> --- Comment #8 from Jonathan Wakely ---
> I think you are still confused between the bool that crate_directory returns,
> and the bool that you get from !ec
>
> The bool that is returned tells you if a new directory was created.
>
> The bool you get from !ec tells you if the directory exists (whether it was
> created or already existed).
I'm afraid I'm still confused. From what you suggest I should be able to write
// return true if p resolves to an existing directory
int
ensure_directories(p)
{
std::error_code ec;
std::filesystem::create_directories("p", ec);
return !ec;
}
But this does not work
$ mkdir xxx
$ ln -sv fff asdf
$ ls asdf
asdf -> xxx
std::filesystem::create_directories("asdf", ec);
ec.value() = 20, !ec = 0
ensure_directories would return false but it shouldn't because asdf
resolves to an existing directory.
[Bug libstdc++/101510] std::filesystem::create_directory on an existing symlink to a directory
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101510 --- Comment #8 from Jonathan Wakely --- I think you are still confused between the bool that crate_directory returns, and the bool that you get from !ec The bool that is returned tells you if a new directory was created. The bool you get from !ec tells you if the directory exists (whether it was created or already existed).
[Bug libstdc++/101510] std::filesystem::create_directory on an existing symlink to a directory
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101510 --- Comment #7 from Madhu --- * "redi at gcc dot gnu.org" Wrote on Tue, 20 Jul 2021 16:00:55 + >> I was going by the usage from databasePath() in >> https://github.com/WebKit/WebKit/blob/main/Source/WebKit/NetworkProcess/ >> WebStorage/LocalStorageDatabaseTracker.cpp#L142 >> which calls ensureDatabaseDirectoryExists() in >> https://github.com/WebKit/WebKit/blob/main/Source/WebCore/platform/sql/ >> SQLiteFileSystem.cpp#L59 >> which calls makeAllDirectories() >> https://github.com/WebKit/WebKit/blob/main/Source/WTF/wtf/FileSystem.cpp#L733 >> >> The check is for only on the boolean value of ec, which is wrong >> according to the spec, Thanks! > > That looks correct to me. It's the same as the example as I showed above. > > If ec does not have a value, it means no error occurred. That means either the > directory was created, or already exists. > >> Instead of precipitately filing the bug here I should have filed it >> with WebKit. > > Are you *actually* seeing incorrect behaviour, or just concluding > there is a bug by reading the code? [There is a problem and I misread the code at first.] The databasePath() function in the first reference consumes a boolean value which is false and incorrectly assumes there is an error. (it correctly reports that the directory could not be created, but then it assumes that the directory does not exist.) This is incorrect behaviour when the name does resolve to a directory The semantics really mean that the boolean value of ec should be ignored for this usage pattern where you want to ensure a directory exists.
[Bug libstdc++/101510] std::filesystem::create_directory on an existing symlink to a directory
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101510 Jonathan Wakely changed: What|Removed |Added Resolution|--- |INVALID Status|WAITING |RESOLVED --- Comment #6 from Jonathan Wakely --- (In reply to Madhu from comment #5) > It looks like I did indeed get the semantics wrong OK, thanks for confirming. > I was going by the usage from databasePath() in > https://github.com/WebKit/WebKit/blob/main/Source/WebKit/NetworkProcess/ > WebStorage/LocalStorageDatabaseTracker.cpp#L142 > which calls ensureDatabaseDirectoryExists() in > https://github.com/WebKit/WebKit/blob/main/Source/WebCore/platform/sql/ > SQLiteFileSystem.cpp#L59 > which calls makeAllDirectories() > https://github.com/WebKit/WebKit/blob/main/Source/WTF/wtf/FileSystem.cpp#L733 > > The check is for only on the boolean value of ec, which is wrong > according to the spec, Thanks! That looks correct to me. It's the same as the example as I showed above. If ec does not have a value, it means no error occurred. That means either the directory was created, or already exists. > Instead of precipitately filing the bug here I should have filed it > with WebKit. Are you *actually* seeing incorrect behaviour, or just concluding there is a bug by reading the code?
[Bug libstdc++/101510] std::filesystem::create_directory on an existing symlink to a directory
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101510
--- Comment #5 from Madhu ---
* "redi at gcc dot gnu.org"
Wrote on Tue, 20 Jul 2021 11:46:32 +
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101510
>
> --- Comment #2 from Jonathan Wakely ---
> (In reply to Madhu from comment #0)
>> cppreference.com states
>>
>> ```
>> bool create_directory( const std::filesystem::path& p );
>>
>> Creates the directory p as if by POSIX mkdir() with a second argument of
>> static_cast(std::filesystem::perms::all) (the parent directory must
>> already exist). If the function fails because p resolves to an existing
>> directory, no error is reported. Otherwise on failure an error is reported.
>> ```
>>
>> This should accomodate situations when `p' resolves to an existing directory
>> when `p' it is a symbolic link.
>>
>> However create_directory(p) fails when p is a symbolic link which points
>> to an existing directory.
>>
>> The standard usage pattern is to call mkdir(p) - and if it fails on EEXIST
>> to stat(2) p - following symlinks, and check if is a directory. The pattern
>> is used to ensure that `p' can be treated as a directory for further
>> operations and this includes p being a symbolic link to a directory)
>
> You can do this with create_directory. If create_directory(p) doesn't throw an
> exception, then is_directory(p) is true. Or without exceptions:
>
> std::error_code ec;
> create_directory(p, ec);
> if (!ec)
> {
> // p resolves to a directory
> }
>
It looks like I did indeed get the semantics wrong
I was going by the usage from databasePath() in
https://github.com/WebKit/WebKit/blob/main/Source/WebKit/NetworkProcess/WebStorage/LocalStorageDatabaseTracker.cpp#L142
which calls ensureDatabaseDirectoryExists() in
https://github.com/WebKit/WebKit/blob/main/Source/WebCore/platform/sql/SQLiteFileSystem.cpp#L59
which calls makeAllDirectories()
https://github.com/WebKit/WebKit/blob/main/Source/WTF/wtf/FileSystem.cpp#L733
The check is for only on the boolean value of ec, which is wrong
according to the spec, Thanks!
Instead of precipitately filing the bug here I should have filed it
with WebKit.
[Bug libstdc++/101510] std::filesystem::create_directory on an existing symlink to a directory
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101510 --- Comment #4 from Jonathan Wakely --- I've added tests to verify that the behaviour is correct, and those tests pass with all versions of GCC.
[Bug libstdc++/101510] std::filesystem::create_directory on an existing symlink to a directory
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101510 --- Comment #3 from CVS Commits --- The master branch has been updated by Jonathan Wakely : https://gcc.gnu.org/g:0c4ae4ff46b1d7633f1e06f57d348b5817b8f640 commit r12-2413-g0c4ae4ff46b1d7633f1e06f57d348b5817b8f640 Author: Jonathan Wakely Date: Tue Jul 20 12:35:37 2021 +0100 libstdc++: Add more tests for filesystem::create_directory [PR101510] Signed-off-by: Jonathan Wakely libstdc++-v3/ChangeLog: PR libstdc++/101510 * src/c++17/fs_ops.cc (create_dir): Adjust whitespace. * testsuite/27_io/filesystem/operations/create_directory.cc: Test creating directory with name of existing symlink to directory. * testsuite/experimental/filesystem/operations/create_directory.cc: Likewise.
[Bug libstdc++/101510] std::filesystem::create_directory on an existing symlink to a directory
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101510
--- Comment #2 from Jonathan Wakely ---
(In reply to Madhu from comment #0)
> cppreference.com states
>
> ```
> bool create_directory( const std::filesystem::path& p );
>
> Creates the directory p as if by POSIX mkdir() with a second argument of
> static_cast(std::filesystem::perms::all) (the parent directory must
> already exist). If the function fails because p resolves to an existing
> directory, no error is reported. Otherwise on failure an error is reported.
> ```
>
> This should accomodate situations when `p' resolves to an existing directory
> when `p' it is a symbolic link.
>
> However create_directory(p) fails when p is a symbolic link which points
> to an existing directory.
>
> The standard usage pattern is to call mkdir(p) - and if it fails on EEXIST
> to stat(2) p - following symlinks, and check if is a directory. The pattern
> is used to ensure that `p' can be treated as a directory for further
> operations and this includes p being a symbolic link to a directory)
You can do this with create_directory. If create_directory(p) doesn't throw an
exception, then is_directory(p) is true. Or without exceptions:
std::error_code ec;
create_directory(p, ec);
if (!ec)
{
// p resolves to a directory
}
[Bug libstdc++/101510] std::filesystem::create_directory on an existing symlink to a directory
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101510 Jonathan Wakely changed: What|Removed |Added Status|ASSIGNED|WAITING --- Comment #1 from Jonathan Wakely --- (In reply to Madhu from comment #0) > However create_directory(p) fails when p is a symbolic link which points > to an existing directory. No it doesn't. It returns false to tell you that it didn't create a directory, but "no error is reported" as required by the specification. Returning false is not reporting an error. If create_directory(p) reports an error then it throws an exception of type filesystem_error, see https://en.cppreference.com/w/cpp/filesystem/create_directory#Exceptions Are you misunderstanding the function's semantics?
[Bug libstdc++/101510] std::filesystem::create_directory on an existing symlink to a directory
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101510 Jonathan Wakely changed: What|Removed |Added Status|UNCONFIRMED |ASSIGNED Assignee|unassigned at gcc dot gnu.org |redi at gcc dot gnu.org Last reconfirmed||2021-07-19 Ever confirmed|0 |1
