| Summary: | Add API test for FileSystem::fileExists() on a broken symbolic link | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||
| Component: | Web Template Framework | Assignee: | Chris Dumez <cdumez> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | achristensen, darin, ggaren, sam, webkit-bug-importer | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=225491 | ||||||
| Attachments: |
|
||||||
|
Description
Chris Dumez
2021-05-06 12:13:21 PDT
Created attachment 427918 [details]
Patch
Comment on attachment 427918 [details]
Patch
Should also add one for a non-broken symlink *to* a broken symlink
(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. 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. Comment on attachment 427918 [details] Patch Clearing flags on attachment: 427918 Committed r277113 (237411@main): <https://commits.webkit.org/237411@main> All reviewed patches have been landed. Closing bug. (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. (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. (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. |