RESOLVED FIXED 225476
Add API test for FileSystem::fileExists() on a broken symbolic link
https://bugs.webkit.org/show_bug.cgi?id=225476
Summary Add API test for FileSystem::fileExists() on a broken symbolic link
Chris Dumez
Reported 2021-05-06 12:13:21 PDT
Add API test for FileSystem::fileExists() on a broken symbolic link.
Attachments
Patch (2.37 KB, patch)
2021-05-06 12:17 PDT, Chris Dumez
no flags
Chris Dumez
Comment 1 2021-05-06 12:17:57 PDT
Darin Adler
Comment 2 2021-05-06 13:09:34 PDT
Comment on attachment 427918 [details] Patch Should also add one for a non-broken symlink *to* a broken symlink
Chris Dumez
Comment 3 2021-05-06 13:11:10 PDT
(In reply to Darin Adler from comment #2) > Comment on attachment 427918 [details] > Patch > > Should also add one for a non-broken symlink *to* a broken symlink For what it's worth FileSystem::exists() for a symlink to a *non-broken* symlink seems to return false. I have noticed this today. I will try and figure out why but the good? news is that it seems this is not new behavior from porting to std::filesystem.
Darin Adler
Comment 4 2021-05-06 13:26:34 PDT
I suppose all these test cases show why we’d rather *not* have our own file system layer. But the convenience of using WTF::String directly is so tempting.
Chris Dumez
Comment 5 2021-05-06 13:27:07 PDT
Comment on attachment 427918 [details] Patch Clearing flags on attachment: 427918 Committed r277113 (237411@main): <https://commits.webkit.org/237411@main>
Chris Dumez
Comment 6 2021-05-06 13:27:10 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 7 2021-05-06 13:28:19 PDT
Chris Dumez
Comment 8 2021-05-06 13:32:51 PDT
(In reply to Darin Adler from comment #4) > I suppose all these test cases show why we’d rather *not* have our own file > system layer. But the convenience of using WTF::String directly is so > tempting. I personally don't mind having our own FileSystem layer with WTF::String convenience and some other small niceties, as long as we keep the WTF::FileSystem layer cross-platform & small (by leveraging std::filesystem) and with a good set of tests. The issue in my opinion was that we had 3 separate implementations with slightly different behaviors and almost no tests.
Chris Dumez
Comment 9 2021-05-06 15:55:58 PDT
(In reply to Chris Dumez from comment #3) > (In reply to Darin Adler from comment #2) > > Comment on attachment 427918 [details] > > Patch > > > > Should also add one for a non-broken symlink *to* a broken symlink > > For what it's worth FileSystem::exists() for a symlink to a *non-broken* > symlink seems to return false. I have noticed this today. I will try and > figure out why but the good? news is that it seems this is not new behavior > from porting to std::filesystem. access() on a symlink to a valid symlink returns -1 and errno is 2, which is: [ENOENT] The named file does not exist.
Chris Dumez
Comment 10 2021-05-06 16:06:58 PDT
(In reply to Chris Dumez from comment #9) > (In reply to Chris Dumez from comment #3) > > (In reply to Darin Adler from comment #2) > > > Comment on attachment 427918 [details] > > > Patch > > > > > > Should also add one for a non-broken symlink *to* a broken symlink > > > > For what it's worth FileSystem::exists() for a symlink to a *non-broken* > > symlink seems to return false. I have noticed this today. I will try and > > figure out why but the good? news is that it seems this is not new behavior > > from porting to std::filesystem. > > access() on a symlink to a valid symlink returns -1 and errno is 2, which is: > [ENOENT] The named file does not exist. Oh, it was an issue with the way FileSystemTest::SetUp() was constructing the symlink. I'll fix that and add more tests shortly.
Note You need to log in before you can comment on or make changes to this bug.