feat: add the ability to override the default temporary directory #286
Conversation
|
(needs test) |
This adds a new `env` module with `override_temp_dir` and `temp_dir` functions. - `temp_dir` will defer to `std::env::temp_dir` by default unless the temporary directory has been overridden. - `override_temp_dir` allows the user to override the default temporary directory ONCE. Once this value has been set, it cannot be changed Care should be taken to ensure that the chosen directory is actually writable. This API is designed for use by the application author when the application may run in an environment without a reliable global temporary directory (e.g., Android). fixes #285
Stebalien
force-pushed
the
steb/override-temp-dir
branch
from
6a12031
to
3d67780
Compare
src/env.rs
Outdated
| /// Only the first call to this function will succeed, returning `Ok(path)`. All further calls will | ||
| /// fail with `Err(path)`. In both cases, `path` is the default temporary directory (on success, the | ||
| /// new one; on failure, the existing one). | ||
| pub fn override_temp_dir(path: &Path) -> Result<&'static Path, &'static Path> { |
There was a problem hiding this comment.
I considered treating this as a fallback (TMPDIR -> the override -> "/tmp"), but handling all the platform-specific cases got hairy.
I also considered testing if the various temporary directories were writable.... but honestly, I don't want to be in the position of "guessing" where temporary files should be written (although I'll revisit that later if this keeps coming up).
There was a problem hiding this comment.
I would suggest Result<(), TempDirAlreadyOverridden> as the return type.
In the success case there is not point in returning the value just passed in.
In the error case it is problematic that Path does not implement Display.
There was a problem hiding this comment.
Docs: Is there a requirement that the directory already exists (I think so) or will it be created (recursively) if non-existent?
There was a problem hiding this comment.
In the success case there is not point in returning the value just passed in.
It returns a static reference, which could be convenient if you need to know where the temporary directory is. But... it's inconsistent with temp_dir, so it's probably not all that useful.
There was a problem hiding this comment.
Hm. Ok, I'm going to change it to return Result<(), PathBuf>, but I actually don't want the user to unwrap() this error, they need to handle it.
src/env.rs
Outdated
| /// Only the first call to this function will succeed, returning `Ok(path)`. All further calls will | ||
| /// fail with `Err(path)`. In both cases, `path` is the default temporary directory (on success, the | ||
| /// new one; on failure, the existing one). | ||
| pub fn override_temp_dir(path: &Path) -> Result<&'static Path, &'static Path> { |
There was a problem hiding this comment.
I would suggest Result<(), TempDirAlreadyOverridden> as the return type.
In the success case there is not point in returning the value just passed in.
In the error case it is problematic that Path does not implement Display.
src/env.rs
Outdated
| /// Only the first call to this function will succeed, returning `Ok(path)`. All further calls will | ||
| /// fail with `Err(path)`. In both cases, `path` is the default temporary directory (on success, the | ||
| /// new one; on failure, the existing one). | ||
| pub fn override_temp_dir(path: &Path) -> Result<&'static Path, &'static Path> { |
There was a problem hiding this comment.
Docs: Is there a requirement that the directory already exists (I think so) or will it be created (recursively) if non-existent?
|
Generally I am not sure if it makes sense to limit the override function to be only called once. Is there a technical reason why you want to prevent it? It was possible before to call For example, a system utility running during boot (initrd) might need to change its temporary directory after mounting some file systems. |
|
I can relax the constraints later if someone presents a strong use-case, but tightening them isn't possible so I'm starting with the strongest constraints. My reason is that In general, applications needing to put specific files in specific directories should use
Init systems re-exec themselves for this reason. Changing the tempdir at runtime likely isn't going to have the desired result (old temporary files will still live in the old location). |
|
Merging this for now. I'll release at the end of the week so we can continue our discussion until then (and possibly iterate on the design). |
This adds a new
envmodule withoverride_temp_dirandtemp_dirfunctions.temp_dirwill defer tostd::env::temp_dirby default unless the temporary directory has been overridden.override_temp_dirallows the user to override the default temporary directory ONCE. Once this value has been set, it cannot be changed Care should be taken to ensure that the chosen directory is actually writable.This API is designed for use by the application author when the application may run in an environment without a reliable global temporary directory (e.g., Android).
fixes #285