-
Notifications
You must be signed in to change notification settings - Fork 561
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
io_unix test leaves stray tmp file #20179
Conversation
@kenneth-olwing you pushed a commit with an updated message to your blead branch, it should've been done on your ken1-io-unix branch. This PR still has the commits with the 'TBD' message.. |
It didn't for me. Testing this revealed a problem with win32_stat(): it fails on socket files, which might be the cause of this problem. |
Damn it, sorry about pushing to the wrong branch, mea culpa. But given that it's marked(?) as a socket file explains why unlink() can't seem to handle it. A perhaps more sane approach might be simply to ignore the test on Windows. While it appears to work, is the concept really applicable (and supported)? And I'm not really crazy about the 'fix' anyway even if it was the only way I could get it to work. I'll fix the branch, but if there is any likelihood of fixing things so unlink() won't choke on it, that would certainly be better (but a bit beyond my means I'm afraid). If any of you have a suggestion on the best way to move I'd appreciate it. As noted it is no biggie, just a bit unclean to have things popping up in the repo and not being ignored by git after a build. Thanks for input, |
I'm closing this for now since I wrecked my commit, but I'd like to hear what would be a better approach, and then I'll make a new branch+PR. |
I expect this needs to be fixed in the core code, at least in win32_stat() and maybe in Perl_apply() (which does unlink() amongst other ops). I'll be looking at win32_stat() tomorrow, but being windows, I expect it to be moderately ugly (see golang/go#33357 for example). |
Ok, awesome. With luck this will 'automatically' (hohum) sort itself out then. The root problem is of course that adapting a concept from *nix it didn't have makes things hurt. I guess it would be equally icky to implement support for something very win specific on *nix. It's still interesting to see that according to the link, the API(s) actually does seem to cater for this, I didn't expect that. I'll be happy to help in any way I'm capable, if it means anything... |
On my Windows 7 system, I get a completely different reaction to the running of t/io_unix.t:
@kenneth-olwing, is it just that you're running a newer version of Windows ? BTW, I noticed tonight that the IO-1.50 distro on CPAN is different to the IO-1.50 in blead. Cheers, |
@sisyphus, yes looking at that golang link it's evident that MS is trying to add unixisms (considering WSL, not that strange I suppose). I doubt any of that exists in Win7, only Win10 and later. Interesting to see how far they will get... |
Yes, AF_UNIX sockets is a Windows 10 feature, see https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/ |
On Windows, the dist/IO/t/io_unix.t test leaves a temporary file 'dist/IO/t/sock-*' like:
../dist/IO/t/io_unix.t .. 1/5 Can't unlink sock-27804: Invalid argument at t/io_unix.t line 103.
The code uses fork() and considering the fork() emulation on Windows and how Windows dislikes removing files that are in use, this causes problems during the attempted unlink() call. I can't figure out exactly why perl can't unlink it later, but doing at the end of the script with an external shell does the trick.
Obviously not a biggie but nice to get the test output cleaner if nothing else...
Better commented commit forthcoming asap.