Add Libc.mkfifo #34587
Add Libc.mkfifo #34587
Conversation
|
Not sure why this is a release blocker? By all means, if the comments are addressed and this gets merged before May 4th, then it will be included. |
|
@vtjnash @JeffBezanson Thanks for the review. I incorporated your comments. |
|
Looking good... All that green! @JeffBezanson want to merge if you're happy here? |
StefanKarpinski
added
the
status:triage
|
Just needs a rebase and to change the error message to ENOT_SUP: kernel does not support fifo |
vtjnash
added
the
status:merge me
base/libc.jl
Outdated
| systemerror("mkfifo", ret == -1) | ||
| return path | ||
| else | ||
| systemerror("mkfifo", ENOTSUP) |
There was a problem hiding this comment.
BTW, I noticed that ENOTSUP doesn't exist in my local build. I guess it's OK as long as this is defined in Windows, though.
There was a problem hiding this comment.
Looks like Linux defines EOPNOTSUP instead, and our regex isn't smart enough to pick up the value for ENOTSUP from that. But CI seems to be okay with this.
There was a problem hiding this comment.
It seems Windows prints ENOTSUP as "Unknown error":
Error in testset file: Test Failed at C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\file.jl:1635 Expression: Libc.mkfifo(joinpath(pwd(), "dummy_path")) Expected: "mkfifo: Operation not supported" Message: "SystemError: mkfifo: Unknown error"
So, I suggest simply doing:
Lines 426 to 428 in 02961cc
Is it OK?
It seems Windows prints it as "Unknown error":
Error in testset file:
Test Failed at C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\file.jl:1635
Expression: Libc.mkfifo(joinpath(pwd(), "dummy_path"))
Expected: "mkfifo: Operation not supported"
Message: "SystemError: mkfifo: Unknown error"
https://build.julialang.org/#/builders/65/builds/5360
This reverts commit 1ff5cd7.
|
As I just needed this functionality, I was going to reinvent the wheel when I stumbled across this PR. I do not understand why it hasn't been merged. Is there a blocker in general? Otherwise, if we prepared that for 1.10 or 1.11, would it be mergeable? |
vtjnash
added
the
status:merge me
|
I just got lost because the merge me tag was removed. Putting that back now. |
IanButterworth
removed
the
status:merge me
|
|
||
| `mkfifo` is supported only in Unix platforms. | ||
|
|
||
| !!! compat "Julia 1.8" |
There was a problem hiding this comment.
Has this been corrected? Obviously this merge was a bit late for 1.8.
There was a problem hiding this comment.
Good catch. Please PR to fix
This PR adds a
mkfifowrapperLibc.mkfifofor Unix platforms.Python has it in
osstdlib and it's useful sometimes. I think it'd be nice if Julia has it in Base/stdlib too.