Remove tempfile dependency

[andreeaflorescu]

The tempfile crate adds 17 dependencies and it is only used in tests. Instead of using tempfile we can create a tiny helper that handles creating files and deleting them when the object gets out of scope.

Functionality needed by Firecracker:

• create directories
• create files

[forbesmyester]

I'd like to take on this issue unless anyone else isn't working on it.

Might take me some time though as I'm still quite a beginner on Rust but I've looked at how tempfile itself works and how it's used in Firecracker and I think I understand what needs doing.

[andreeaflorescu]

Hey @forbesmyester you can take the issue.

When creating a tempfile replacement please keep in mind that we don't want to use any other external dependencies. You will need to get a random file/directory name using other methods. There is already a mechanism implemented in Firecracker for getting random numbers without using the rand crate. @alexandruag has more details on this one.

[forbesmyester]

Thanks

Sure I get the whole point of this is to remove dependencies :-)

The plan is to replicate the methods used in tempfile for the functions firecracker is using as it would seem Firecracker is happy with the functionality, but do sensible substitutions like the rand issue you mention ( which I think is visible in 7fba4f0 ).

[andreeaflorescu]

@forbesmyester awesome research! Let us know if you need help with anything.

[andreeaflorescu]

@forbesmyester are you still working on this issue?

We will use rust-vmm components after they're published to crates.io. In the vmm-sys-utils crate there is already an implementation for tempdir and I think it's better to add the missing functionality there as well as more projects can benefit from this.

[forbesmyester]

Hi @andreeaflorescu

Yes I am. I think I'm practically done but I need to:

• sort out my commits (squash etc)
• I have questions about how much of the documentation from tempfile to bring through.
• how much of the API which is not currently used to bring over too.
• I need to look at the .with_err_path(|| path.clone()) and figure out if that's required and how to bring it over and test if I do.
• Integrate it, should be easy as I intend to mirror the API.

My commits are currently a mess ( I'll need to squash etc) and there's the above issues but I think what I've got is actually pretty good.

Currently code located in https://github.com/forbesmyester/firecracker/blob/master/fc_util/src/fs.rs . I've not started any integration yet so if we want to move it now's a good time.

[andreeaflorescu]

I'll say to submit a PR directly in vmm-sys-utils.

CC: @firecracker-microvm/compute-capsule anyone against this?

[forbesmyester]

Hi All,

Looking at vmm_sys_util it seems they already have a tempdir and it uses the libc function mkdtemp, which I didn't know about, but probably should have.

As to our usage of tempfile.

We're using tempdir_in once, which we'd probably want to port to afforementioned vmm-sys-util/tempdir.rs... They're remarkably similar, though tempdir supports more OS's they resolve to practically the same thing on Linux.

In tempfile any creation of a temporary directory just loops back around to tempdir_in using &env::temp_dir() as the location and ends up creating a TempDir. All the directory functions in tempfile and vmm_sys_util clean up on drop, as apposed to having the OS do it, so there's no difference there either.

Therefore any usage for creating temporary directories already seems to be covered by vmm_sys_util.

Looking at the file front. tempfile has two ways.

1. One is the struct NamedTempFile which will create a file and returns a struct . Like TempDir it will clean up on the drop.
2. The other is create_unlinked which creates a file which is immediately deleted. It returns a File but to this deleted file (which is fine).

I think we ought to propose a patch to vmm_sys_util which uses the libc function mkstemp and is the mirror of their TempDir. This covers of the first construct.

For the second it would seem we should just wrap the first? But do you think we should do the wrapping here or there?

What are your thoughts?

[andreeaflorescu]

@forbesmyester as far as I remember we only needed a NamedTempFile because there was no way to get the path of a TempFile which was needed for some tests. If we can have a way to get the path of the file, I think we can leave with only one type of temporary file.

Tempfile and tempdir are used in tests in other rust-vmm components so I think we should add this functionality to rust-vmm so we can remove the dependency on tempfile. See this issue: rust-vmm/vm-memory#23

I wouldn't stress too much on creating an API for tempfile and tempdir that matches exactly the ones that we are using from the tempfile crate. I think it's more of a nice to have, but it shouldn't be mandatory.