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

Design question: why support just dirs and not files? #19

Closed
oconnor663 opened this Issue Jan 23, 2017 · 16 comments

Comments

Projects
None yet
6 participants
@oconnor663

oconnor663 commented Jan 23, 2017

I'm curious about the reasons for focusing specifically on temp dirs and not temp files. Are there security benefits to always using a dir? Or simplicity/portability benefits? Asking out of historical interest more than anything else. (Apologies if this is an abuse of issues :) )

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton Jan 23, 2017

Member

At this point it's mostly just historical and not intentional. Long ago we had TempDir in the standard library, then the compiler, and then this external crate. I don't think this has been actively developed since its inception really, and a PR to implement temp files would be most welcome!

Member

alexcrichton commented Jan 23, 2017

At this point it's mostly just historical and not intentional. Long ago we had TempDir in the standard library, then the compiler, and then this external crate. I don't think this has been actively developed since its inception really, and a PR to implement temp files would be most welcome!

@oconnor663

This comment has been minimized.

Show comment
Hide comment
@oconnor663

oconnor663 Jan 23, 2017

Ooo good to know. I bet a lot of @Stebalien's work on https://stebalien.github.io/tempfile/tempfile could be imported. (There are a bunch of places in those docs where it says "SECURITY WARNING" though, will want to think carefully about those one way or another.)

Edit: relevant context https://www.reddit.com/r/rust/comments/32n864/tempfile_temporary_file_library/cqd3ic0/?context=3

oconnor663 commented Jan 23, 2017

Ooo good to know. I bet a lot of @Stebalien's work on https://stebalien.github.io/tempfile/tempfile could be imported. (There are a bunch of places in those docs where it says "SECURITY WARNING" though, will want to think carefully about those one way or another.)

Edit: relevant context https://www.reddit.com/r/rust/comments/32n864/tempfile_temporary_file_library/cqd3ic0/?context=3

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton Jan 23, 2017

Member

Sounds plausible to me!

Member

alexcrichton commented Jan 23, 2017

Sounds plausible to me!

@opilar

This comment has been minimized.

Show comment
Hide comment
@opilar

opilar May 12, 2017

Contributor

If we want to merge tempdir and tempfile would it be better to call it temp crate?

Contributor

opilar commented May 12, 2017

If we want to merge tempdir and tempfile would it be better to call it temp crate?

@KodrAus

This comment has been minimized.

Show comment
Hide comment
@KodrAus

KodrAus Aug 3, 2017

Contributor

Is this something you guys would like to do before a 1.0 release? Or do you think it's sufficient to have an example somewhere using both tempdir and tempfile?

cc: @alexcrichton

Contributor

KodrAus commented Aug 3, 2017

Is this something you guys would like to do before a 1.0 release? Or do you think it's sufficient to have an example somewhere using both tempdir and tempfile?

cc: @alexcrichton

@oconnor663

This comment has been minimized.

Show comment
Hide comment
@oconnor663

oconnor663 Aug 3, 2017

It seems like most other languages I've used handle both in one library? New users might find it a little surprising to split them?

https://docs.python.org/3.6/library/tempfile.html
https://golang.org/pkg/io/ioutil/

oconnor663 commented Aug 3, 2017

It seems like most other languages I've used handle both in one library? New users might find it a little surprising to split them?

https://docs.python.org/3.6/library/tempfile.html
https://golang.org/pkg/io/ioutil/

@aturon

This comment has been minimized.

Show comment
Hide comment
@aturon

aturon commented Aug 3, 2017

cc me

@Stebalien

This comment has been minimized.

Show comment
Hide comment
@Stebalien

Stebalien Aug 3, 2017

SECURITY WARNING

Basically, relying on named temporary files in the presence of temporary file cleaners is tricky. However, to be honest, I could probably soften those warnings.

If we want to merge tempdir and tempfile would it be better to call it temp crate?

I'd also be happy to give up the tempfile crate name for the greater good. Alternatively, someone could write an fsutil crate for all these miscellaneous filesystem related tools.

Stebalien commented Aug 3, 2017

SECURITY WARNING

Basically, relying on named temporary files in the presence of temporary file cleaners is tricky. However, to be honest, I could probably soften those warnings.

If we want to merge tempdir and tempfile would it be better to call it temp crate?

I'd also be happy to give up the tempfile crate name for the greater good. Alternatively, someone could write an fsutil crate for all these miscellaneous filesystem related tools.

@KodrAus

This comment has been minimized.

Show comment
Hide comment
@KodrAus

KodrAus Aug 3, 2017

Contributor

I can think of a few paths forward:

  1. Do nothing
  2. Deprecate tempdir or tempfile and merge them into a single crate (in this case I think it's worth pointing out that tempfile is stable and tempdir isn't, but tempdir has more dependent crates)
  3. Add some docs/examples to tempdir and tempfile but keep the crates separate
  4. Keep the crates separate, but make another crate, temp or fsutil, or whatever that re-exports or wraps bits of tempdir and tempfile, maybe dealing with issues like conflicting destructors

I guess which option makes the most sense depends on how important this is, and whether we're happy to defer stabilising tempdir until there's a crate that offers both temporary files and temporary directories.

Maybe we should cast the net wider and see if other Rustacean's expect temporary file and directory handling in a single crate?

What do you all think?

Contributor

KodrAus commented Aug 3, 2017

I can think of a few paths forward:

  1. Do nothing
  2. Deprecate tempdir or tempfile and merge them into a single crate (in this case I think it's worth pointing out that tempfile is stable and tempdir isn't, but tempdir has more dependent crates)
  3. Add some docs/examples to tempdir and tempfile but keep the crates separate
  4. Keep the crates separate, but make another crate, temp or fsutil, or whatever that re-exports or wraps bits of tempdir and tempfile, maybe dealing with issues like conflicting destructors

I guess which option makes the most sense depends on how important this is, and whether we're happy to defer stabilising tempdir until there's a crate that offers both temporary files and temporary directories.

Maybe we should cast the net wider and see if other Rustacean's expect temporary file and directory handling in a single crate?

What do you all think?

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton Aug 5, 2017

Member

I'd be totally down for merging the two crates somehow if desired, it's sort of what I'd expect! I think tempfiles have the nice property that they can almost always have guaranteed cleanup on process exit, whereas temporary directories not so much :(

Member

alexcrichton commented Aug 5, 2017

I'd be totally down for merging the two crates somehow if desired, it's sort of what I'd expect! I think tempfiles have the nice property that they can almost always have guaranteed cleanup on process exit, whereas temporary directories not so much :(

@KodrAus

This comment has been minimized.

Show comment
Hide comment
@KodrAus

KodrAus Aug 7, 2017

Contributor

Having the one crate with a consistent API for temporary directories and files does sound nice.

I'm just not sure how much we should churn users of tempdir or tempfile. @Stebalien I don't think anyone's actually properly asked you what you think and whether you'd be on board with merging these crates in some way.

Contributor

KodrAus commented Aug 7, 2017

Having the one crate with a consistent API for temporary directories and files does sound nice.

I'm just not sure how much we should churn users of tempdir or tempfile. @Stebalien I don't think anyone's actually properly asked you what you think and whether you'd be on board with merging these crates in some way.

@Stebalien

This comment has been minimized.

Show comment
Hide comment
@Stebalien

Stebalien Aug 7, 2017

whether you'd be on board with merging these crates in some way.

Completely on board.

However, I'd really like a directory abstraction that allows one to securely create files/directories under a directory (guaranteeing that the files are actually created under the given directory) and have TempDir dereference to said directory abstraction. For example, on Linux, Directory would keep a file descriptor open for the directory and use openat/mkdirat to create files under it. On macos, one can use per-thread current working directories (😈) to achieve the same effect (at the cost of two additional syscalls...).

Stebalien commented Aug 7, 2017

whether you'd be on board with merging these crates in some way.

Completely on board.

However, I'd really like a directory abstraction that allows one to securely create files/directories under a directory (guaranteeing that the files are actually created under the given directory) and have TempDir dereference to said directory abstraction. For example, on Linux, Directory would keep a file descriptor open for the directory and use openat/mkdirat to create files under it. On macos, one can use per-thread current working directories (😈) to achieve the same effect (at the cost of two additional syscalls...).

@KodrAus

This comment has been minimized.

Show comment
Hide comment
@KodrAus

KodrAus Aug 10, 2017

Contributor

It looks like this is something folks are pretty much on-board with, but it's a pretty significant undertaking. Would a good next step be a pre-RFC to work out specifics like crate name and API?

Contributor

KodrAus commented Aug 10, 2017

It looks like this is something folks are pretty much on-board with, but it's a pretty significant undertaking. Would a good next step be a pre-RFC to work out specifics like crate name and API?

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton Aug 10, 2017

Member

I wouldn't necessarily think this needs an RFC, but it probably couldn't hurt if you're feeling up to the task!

Member

alexcrichton commented Aug 10, 2017

I wouldn't necessarily think this needs an RFC, but it probably couldn't hurt if you're feeling up to the task!

@KodrAus

This comment has been minimized.

Show comment
Hide comment
@KodrAus

KodrAus Aug 10, 2017

Contributor

Sure, I'll get the ball rolling on this 👍

Contributor

KodrAus commented Aug 10, 2017

Sure, I'll get the ball rolling on this 👍

opilar pushed a commit to opilar/tempdir that referenced this issue Sep 22, 2017

Added contents_first option (rust-lang-deprecated#19)
Added contents_first option

Added ability to yield the contents of the directory before the
directory itself

Fixes rust-lang-deprecated#18

@KodrAus KodrAus referenced this issue Dec 1, 2017

Closed

Library evaluation tracking issue #28

6 of 7 tasks complete
@KodrAus

This comment has been minimized.

Show comment
Hide comment
@KodrAus

KodrAus Feb 7, 2018

Contributor

We've now got the tempdir API merged into tempfile 🎉

I'll go ahead and close this now, and we can continue tracking the progress of a new tempfile release including temporary files and directories in that repository.

Contributor

KodrAus commented Feb 7, 2018

We've now got the tempdir API merged into tempfile 🎉

I'll go ahead and close this now, and we can continue tracking the progress of a new tempfile release including temporary files and directories in that repository.

@KodrAus KodrAus closed this Feb 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment