-
Notifications
You must be signed in to change notification settings - Fork 654
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
NIOFileSystem.temporaryDirectory: Consider falling back to TMPDIR #2861
Comments
That seems pretty reasonable to me. I wonder if |
For reference this is what |
I would think Besides the "expressed preference" comment from @glbrntt, there are other considerations:
For completeness, I can also see some arguments for aligning with |
CC @parkera also |
swift-foundation should be able to accommodate both use cases, and it would be nice to have a single implementation of this kind of logic. As you can see from the one we have there already, it is unfortunately not trivial. It's also something which varies dramatically depending on platform, so abstracting it once also makes sense. |
Motivation: This PR aligns what temp directory NIOFileSystem will return first: - First try `${TMPDIR}` as it's an expressed user preference - On Darwin, try `_CS_DARWIN_USER_TMP_DIR` next. If it's set, return that. - Finally, fall back on `/tmp` on Darwin and Linux, and `/data/local/tmp` on Android. Closes apple#2861. Modifications: - Reworks `NIOFileSystem.temporaryDirectory`. Result: - NIOFileSystem will now try `${TMPDIR}` first for the temporary directory. Caveats: - We might want to align how Swift-NIO and FoundationEssentials resolve temp directory, and [FoundationEssentials have a different approach](https://github.com/swiftlang/swift-foundation/blob/9d57f36de757b3d5e3a2f7ffcf27aaec3033509f/Sources/FoundationEssentials/String/String%2BPath.swift#L484-L533). But, I agree with [this comment](apple#2861 (comment)), it feels like TMPDIR is an expected default.
swift-nio/Sources/NIOFileSystem/FileSystem.swift
Line 638 in 30df855
Currently, NIOFS's
temporaryDirectory
uses_CS_DARWIN_USER_TEMP_DIR
on Darwin and/tmp
//data/local/tmp
anywhere else. I think we should consider the env varTMPDIR
if set.So maybe:
_CS_DARWIN_USER_TEMP_DIR
->TMPDIR
->/tmp
TMPDIR
->/tmp
TMPDIR
->/data/local/tmp
?
The text was updated successfully, but these errors were encountered: