Stebalien / tempfile

Temporary file library for rust
http://stebalien.com/projects/tempfile-rs
Apache License 2.0
1.15k stars 113 forks source link

NamedTempFile does not remove file if being written to #263

Closed MaxG87 closed 10 months ago

MaxG87 commented 10 months ago

Today I started to work on my first meaningful Rust project. I want to write a small CLI to convert some file types. For this I want to

  1. have a test where I provide input and output file
  2. have the CLI write to the output file
  3. compare the output file with some expected file
  4. cleanup

Points 1-3 work very well already.

I use tempfile to generate the output file. Since I have to pass the file name to the CLI under test, I use tempfile::NamedTempFile. More precisely, I use tempfile::Builder, because I need to control the suffix. However, that wouldn't change anything.

As soon as the NamedTempFile is written to, it is not tidied up. I found that quite surprising and I couldn't find a hint on that in the documentation. I wanted to raise this and clarify if this is intended behaviour. If so, I would be very glad if there could be an option to control that behaviour.

In case a reproducing program is required. I created one. I would expect it to leave no trace, but instead a file remains.

use std::fs::File;
use std::io::Write;
use std::path::PathBuf;

use tempfile::NamedTempFile;

fn main() {
    let named_tempfile = NamedTempFile::new().unwrap();

    let result = named_tempfile
        .path()
        .file_name()
        .map(PathBuf::from)
        .unwrap();

    let mut buffer = File::create(&result).unwrap();
    buffer.write_all(b"Hello, world!").unwrap();
}
Stebalien commented 10 months ago

You're:

  1. Creating a named tempfile.
  2. Extracting the path.
  3. Dropping the tempfile (deleting it).
  4. Creating a new file in File::create.

You want:

use std::fs::File;
use std::io::Write;
use std::path::PathBuf;

use tempfile::NamedTempFile;

fn main() {
    let named_tempfile = NamedTempFile::new().unwrap();

    named_tempfile.write_all(b"Hello, world!").unwrap();
}
MaxG87 commented 10 months ago

Thank you for your quick reply. I am very sorry to reopen it, but I don't understand some aspects.

First, I don't even see where named_tempfile is dropped. It is assigned, not moved (.path() borrows it) and stays in scope until the end of the block. How can it be "dropped"?

Second, I want to use a NamedTempFile so I can pass the file name around and have the file itself being deleted at the end of the test. I have a heavy Python background, so maybe that influences my design. In Python, I would do:

with tempfile.NamedTemporaryFile as ntf:
    do_stuff(ntf.name)
    # File gets deleted automatically when leaving the scope of the block.
# ntf has vanished here

While your new snippet manages to write into the temporary file, I cannot learn from it how I could pass the file's name to another function. Could you help me there, please?

Stebalien commented 10 months ago

First, I don't even see where named_tempfile is dropped. It is assigned, not moved (.path() borrows it) and stays in scope until the end of the block. How can it be "dropped"?

Ah, you're right. The usual issue is NamedTempFile::new().unwrap().path().into_owned() immediately dropping the temporary file.

Here, I'm guessing your issue is that file_name is returning a file name relative to your current directory, while NamedTempFile::new() creates a temporary file in your default temporary file directory (e.g., /tmp).

While your new snippet manages to write into the temporary file, I cannot learn from it how I could pass the file's name to another function. Could you help me there, please?

You can still pass the path to another program, my point was that you shouldn't re-create the file with File::create.

MaxG87 commented 10 months ago

Thank you for your pointer. I figured out what I have to do and pasting it here in case someone directed here by search engines drops in.

What I did was let result = named_tempfile.path().file_name().map(PathBuf::from).unwrap();. The correct way way is let result_fname = named_tempfile.path().to_str().unwrap();.

First, I got rid of the misconception. Of course (in hindsight) file_name() does not help to extract the path as a &str. It returns the last component of the path. Thus, with that I changed the path.

Second, trying to use PathBuf was complicating things. Using to_str() is sufficient here, the PathBuf type does not add any value in the particular case I work on.

Stebalien commented 10 months ago

I'd be a bit careful with that as &str cannot represent all valid paths (at least on unix-like operating systems). The "correct" way to do this is named_tempfile.path().to_owned().