225476 2021-05-06 12:13:21 -0700 Add API test for FileSystem::fileExists() on a broken symbolic link 2021-05-06 16:14:26 -0700 1 1 1 Unclassified WebKit Web Template Framework WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=225491 InRadar P2 Normal --- 1 cdumez cdumez achristensen darin ggaren sam webkit-bug-importer oldest_to_newest 1757565 0 cdumez 2021-05-06 12:13:21 -0700 Add API test for FileSystem::fileExists() on a broken symbolic link. 1757568 1 427918 cdumez 2021-05-06 12:17:57 -0700 Created attachment 427918 Patch 1757586 2 427918 darin 2021-05-06 13:09:34 -0700 Comment on attachment 427918 Patch Should also add one for a non-broken symlink *to* a broken symlink 1757588 3 cdumez 2021-05-06 13:11:10 -0700 (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. 1757597 4 darin 2021-05-06 13:26:34 -0700 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. 1757599 5 427918 cdumez 2021-05-06 13:27:07 -0700 Comment on attachment 427918 Patch Clearing flags on attachment: 427918 Committed r277113 (237411@main): <https://commits.webkit.org/237411@main> 1757600 6 cdumez 2021-05-06 13:27:10 -0700 All reviewed patches have been landed. Closing bug. 1757602 7 webkit-bug-importer 2021-05-06 13:28:19 -0700 <rdar://problem/77622567> 1757606 8 cdumez 2021-05-06 13:32:51 -0700 (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. 1757678 9 cdumez 2021-05-06 15:55:58 -0700 (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. 1757688 10 cdumez 2021-05-06 16:06:58 -0700 (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. 427918 2021-05-06 12:17:57 -0700 2021-05-06 13:27:07 -0700 Patch bug-225476-20210506121756.patch text/plain 2430 cdumez U3VidmVyc2lvbiBSZXZpc2lvbjogMjc3MTAxCmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggMzhjNTk1ODQzOWU5NmUyYzRjNDQ0MzM1YmZkNTA4YjE3 ZGQxYzZhYi4uZjgyN2FlYWE1M2Y3MDkxZTE5ZWJlNzFmZmJjMjFmZTY4ZTIyN2JmZCAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4 IEBACisyMDIxLTA1LTA2ICBDaHJpcyBEdW1leiAgPGNkdW1lekBhcHBsZS5jb20+CisKKyAgICAg ICAgQWRkIEFQSSB0ZXN0IGZvciBGaWxlU3lzdGVtOjpmaWxlRXhpc3RzKCkgb24gYSBicm9rZW4g c3ltYm9saWMgbGluaworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5j Z2k/aWQ9MjI1NDc2CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAg ICAgICAgQWRkIEFQSSB0ZXN0IGZvciBGaWxlU3lzdGVtOjpmaWxlRXhpc3RzKCkgb24gYSBicm9r ZW4gc3ltYm9saWMgbGluayBzaW5jZSB0aGlzIGlzIGEgc3BlY2lhbAorICAgICAgICBjYXNlLiBG aWxlU3lzdGVtOjpmaWxlRXhpc3RzKCkgY3VycmVudGx5IHRyaWVzIHRvIGZvbGxvdyBzeW1ib2xp YyBsaW5rcyBhbmQgd2lsbCB0aHVzIHJldHVybgorICAgICAgICBmYWxzZS4gVGhlIGJlaGF2aW9y IGlzIGEgbGl0dGxlIG9kZCBidXQgSSBoYXZlIHZlcmlmaWVkIHRoYXQgdGhpcyBpcyB3aGF0IG91 ciBhY2Nlc3MoKS1iYXNlZAorICAgICAgICBpbXBsZW1lbnRhdGlvbiB3YXMgcmV0dXJuaW5nIGFs c28gYmVmb3JlIEkgcG9ydGVkIEZpbGVTeXN0ZW06OmZpbGVFeGlzdHMoKSB0byBzdGQ6OmZpbGVz eXN0ZW0uCisKKyAgICAgICAgKiBUZXN0V2ViS2l0QVBJL1Rlc3RzL1dURi9GaWxlU3lzdGVtLmNw cDoKKyAgICAgICAgKFRlc3RXZWJLaXRBUEk6OlRFU1RfRik6CisKIDIwMjEtMDUtMDQgIENocmlz IER1bWV6ICA8Y2R1bWV6QGFwcGxlLmNvbT4KIAogICAgICAgICBQb3J0IEZpbGVzeXN0ZW06OmZp bGVNZXRhZGF0YSgpICYgRmlsZXN5c3RlbTo6Z2V0RmlsZU1vZGlmaWNhdGlvblRpbWUoKSB0byBz dGQ6OmZpbGVzeXN0ZW0KZGlmZiAtLWdpdCBhL1Rvb2xzL1Rlc3RXZWJLaXRBUEkvVGVzdHMvV1RG L0ZpbGVTeXN0ZW0uY3BwIGIvVG9vbHMvVGVzdFdlYktpdEFQSS9UZXN0cy9XVEYvRmlsZVN5c3Rl bS5jcHAKaW5kZXggOWRhNjYzOTkyYmUwOWE2Y2Y0Y2MxMzhiYTc2ZGNjYmNmMGZkNzc4MS4uMWI0 YjA5ZmZmYjRmNDAwYmMzZGMyNmY3MTEwODkwYjNkN2FkYjExYiAxMDA2NDQKLS0tIGEvVG9vbHMv VGVzdFdlYktpdEFQSS9UZXN0cy9XVEYvRmlsZVN5c3RlbS5jcHAKKysrIGIvVG9vbHMvVGVzdFdl YktpdEFQSS9UZXN0cy9XVEYvRmlsZVN5c3RlbS5jcHAKQEAgLTI3OSw2ICsyNzksMTkgQEAgVEVT VF9GKEZpbGVTeXN0ZW1UZXN0LCBmaWxlRXhpc3RzKQogICAgIEVYUEVDVF9GQUxTRShGaWxlU3lz dGVtOjpmaWxlRXhpc3RzKEZpbGVTeXN0ZW06OnBhdGhCeUFwcGVuZGluZ0NvbXBvbmVudCh0ZW1w RW1wdHlGb2xkZXJQYXRoKCksICJkb2VzLW5vdC1leGlzdCJfcykpKTsKIH0KIAorVEVTVF9GKEZp bGVTeXN0ZW1UZXN0LCBmaWxlRXhpc3RzQnJva2VuU3ltbGluaykKK3sKKyAgICBhdXRvIGRvZXNO b3RFeGlzdFBhdGggPSBGaWxlU3lzdGVtOjpwYXRoQnlBcHBlbmRpbmdDb21wb25lbnQodGVtcEVt cHR5Rm9sZGVyUGF0aCgpLCAiZG9lcy1ub3QtZXhpc3QiX3MpOworICAgIGF1dG8gc3ltbGlua1Bh dGggPSBGaWxlU3lzdGVtOjpwYXRoQnlBcHBlbmRpbmdDb21wb25lbnQodGVtcEVtcHR5Rm9sZGVy UGF0aCgpLCAiZG9lcy1ub3QtZXhpc3Qtc3ltbGluayJfcyk7CisgICAgRVhQRUNUX1RSVUUoRmls ZVN5c3RlbTo6Y3JlYXRlU3ltYm9saWNMaW5rKGRvZXNOb3RFeGlzdFBhdGgsIHN5bWxpbmtQYXRo KSk7CisgICAgRVhQRUNUX0ZBTFNFKEZpbGVTeXN0ZW06OmZpbGVFeGlzdHMoZG9lc05vdEV4aXN0 UGF0aCkpOworICAgIEVYUEVDVF9GQUxTRShGaWxlU3lzdGVtOjpmaWxlRXhpc3RzKHN5bWxpbmtQ YXRoKSk7IC8vIGZpbGVFeGlzdHMoKSBmb2xsb3dzIHN5bWxpbmtzLgorICAgIGF1dG8gc3ltbGlu a01ldGFkYXRhID0gRmlsZVN5c3RlbTo6ZmlsZU1ldGFkYXRhKHN5bWxpbmtQYXRoKTsKKyAgICBB U1NFUlRfVFJVRSghIXN5bWxpbmtNZXRhZGF0YSk7CisgICAgRVhQRUNUX0VRKHN5bWxpbmtNZXRh ZGF0YS0+dHlwZSwgRmlsZU1ldGFkYXRhOjpUeXBlOjpTeW1ib2xpY0xpbmspOworICAgIEVYUEVD VF9UUlVFKEZpbGVTeXN0ZW06OmRlbGV0ZUZpbGUoc3ltbGlua1BhdGgpKTsKK30KKwogVEVTVF9G KEZpbGVTeXN0ZW1UZXN0LCBkZWxldGVTeW1saW5rKQogewogICAgIEVYUEVDVF9UUlVFKEZpbGVT eXN0ZW06OmZpbGVFeGlzdHModGVtcEZpbGVQYXRoKCkpKTsK