Implement File.tempfile in Crystal
#12111
Conversation
straight-shoota
changed the title
Crystal::System::File.open variantFile.tempfile in Crystal
|
Somewhat related: #12393. I have a draft PR that also revamps some how files are opened on Windows. I'll have to see if this PR fixes that as well, then could just close that other PR ideally. |
| @@ -10,6 +10,7 @@ lib LibC | |||
| FD_CLOEXEC = 1 | |||
| O_CLOEXEC = 0 | |||
| O_CREAT = 1_u16 << 12 | |||
| O_EXCL = 4_u16 << 12 | |||
There was a problem hiding this comment.
Just asking, did you go through the system headers of all supported systems to grab those O_EXCL values?
There was a problem hiding this comment.
I don't recall doing that explicitly. That's the problem with ridiculously long review timespans (poke @beta-ziliani 😏).
But I'm pretty sure I must have. The values are different, and I don't think I rolled a dice 😆
Perhaps I didn't go through all original headers though and grabbed the values from some other implementation in Go instead. Can't say.
There was a problem hiding this comment.
This is the WASI definition I could find: https://github.com/WebAssembly/wasi-libc/blob/8daaba387ce70a6088afe3916e9e16ed1c2bc630/libc-bottom-half/headers/public/__header_fcntl.h
Then it looks like Linux uses 0200 whereas the BSDs (including Darwin) use 0x0800. So those values are probably correct
| @@ -47,6 +47,43 @@ module Crystal::System::File | |||
| m | o | |||
| end | |||
|
|
|||
| LOWER_ALPHANUM = "0123456789abcdefghijklmnopqrstuvwxyz".to_slice | |||
|
|
|||
| def self.mktemp(prefix : String?, suffix : String?, dir : String, random : ::Random = ::Random::DEFAULT) : {LibC::Int, String} | |||
There was a problem hiding this comment.
Is there a reason the name generation logic here needs to be distinct from ::File.tempname? That method also includes the process ID and the current time. Could we include them here? Could we get rid of them there?
There was a problem hiding this comment.
I think I considered the implications for tempname that but it's probably better done in a follow up to keep things simple.
Co-authored-by: Quinton Miller <[email protected]>
| @@ -47,6 +47,43 @@ module Crystal::System::File | |||
| m | o | |||
| end | |||
|
|
|||
| LOWER_ALPHANUM = "0123456789abcdefghijklmnopqrstuvwxyz".to_slice | |||
There was a problem hiding this comment.
It's in the Crystal::System namespace, so inherently non-public.
This patch implements tempfile creation natively in Crystal, instead of relying on
mkstempon some platforms.We already used a custom implementation on Windows, but it lacked reliability in case of a name collision and produced overly long file names. So the new implementation is a complete rewrite.
Resolves #12082