Skip to content
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

Closed
wants to merge 2 commits into from
Closed

io_unix test leaves stray tmp file #20179

wants to merge 2 commits into from

Conversation

kenneth-olwing
Copy link
Contributor

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.

@bram-perl
Copy link

@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..
Can you also explain what you tested?
What I noticed in the test is that it never does a close $listen so based on that I would guess that $PATH is still in use and that the unlink fails because of that.. but I have not verified that yet..
Can you try adding a close $listen; before the unlink and see if that works?

@tonycoz
Copy link
Contributor

tonycoz commented Aug 31, 2022

Can you try adding a close $listen; before the unlink and see if that works?

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.

@kenneth-olwing
Copy link
Contributor Author

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,
ken1

@kenneth-olwing
Copy link
Contributor Author

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.

@tonycoz
Copy link
Contributor

tonycoz commented Aug 31, 2022

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).

@kenneth-olwing
Copy link
Contributor Author

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...

@sisyphus
Copy link
Contributor

On my Windows 7 system, I get a completely different reaction to the running of t/io_unix.t:

../dist/IO/t/io_unix.t.............. skipped: AF_UNIX unavailable or disabled on this platform

@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.
I've created #20201 to rectify this, but just keep in mind that if you do want to start testing by using the IO-1.50 CPAN source then you should first update that source with the IO/poll.h from blead.

Cheers,
Rob

@kenneth-olwing
Copy link
Contributor Author

@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...

@tonycoz
Copy link
Contributor

tonycoz commented Sep 5, 2022

On my Windows 7 system, I get a completely different reaction to the running of t/io_unix.t:

../dist/IO/t/io_unix.t.............. skipped: AF_UNIX unavailable or disabled on this platform

Yes, AF_UNIX sockets is a Windows 10 feature, see https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants