WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Chris Dumez
Comment 1
2021-05-06 12:17:57 PDT
Created
attachment 427918
[details]
Patch
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
<
rdar://problem/77622567
>
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.
Top of Page
Format For Printing
XML
Clone This Bug