Skip to content
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

Tracking Issue for file_create_new #105135

Closed
2 of 3 tasks
yjhn opened this issue Dec 1, 2022 · 12 comments · Fixed by #119153
Closed
2 of 3 tasks

Tracking Issue for file_create_new #105135

yjhn opened this issue Dec 1, 2022 · 12 comments · Fixed by #119153
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@yjhn
Copy link
Contributor

yjhn commented Dec 1, 2022

Feature gate: #![feature(file_create_new)]

This is a tracking issue for std::fs::File::create_new(path) function. It is related to RFC 1252, but isn't part of the RFC.

Public API

// std::fs

impl File 
    pub fn create_new<P: AsRef<Path>>(path: P) -> io::Result<File>
}

Steps / History

Unresolved Questions

  • None yet.

Footnotes

  1. https://std-dev-guide.rust-lang.org/feature-lifecycle/stabilization.html

@yjhn yjhn added C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Dec 1, 2022
@Noratrieb
Copy link
Member

This was implemented in #98801.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Dec 2, 2022
Add tracking issue number for `file_create_new` feature

It was missing a tracking issue, so I opened one (rust-lang#105135).
@joshtriplett
Copy link
Member

This has been implemented for a while, and seems to work as advertised.

Shall we stabilize it?

@rfcbot merge

@rfcbot
Copy link

rfcbot commented Oct 27, 2023

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Oct 27, 2023
@BurntSushi
Copy link
Member

Is this a common enough routine to need to justify its own constructor on File? Needing to use OpenOptions for a case like this seems okay to me?

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Nov 7, 2023
@rfcbot
Copy link

rfcbot commented Nov 7, 2023

🔔 This is now entering its final comment period, as per the review above. 🔔

@mr-bit
Copy link

mr-bit commented Nov 10, 2023

File::create returns a file in write-only mode but File::create_new returns a file in read-write mode. What is the reason for this difference?

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Nov 17, 2023
@rfcbot
Copy link

rfcbot commented Nov 17, 2023

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@Amanieu Amanieu added the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Nov 19, 2023
@joshtriplett
Copy link
Member

Is this a common enough routine to need to justify its own constructor on File? Needing to use OpenOptions for a case like this seems okay to me?

The use case for create_new seems at least as common as create, in filesystem code that I've observed. create is useful for "the user told me to write to a specific filename, which may or may not exist" (e.g. shell redirection or a -o option). create_new is used when writing out a file expected to not exist, where using create might introduce a bug and potentially a security vulnerability. Having only create and not create_new seems likely to steer people towards create even if it's not what they should be using, which would make such bugs more likely. Having both makes it obvious that the user needs to select the appropriate one for their use case.

@BurntSushi
Copy link
Member

@joshtriplett I buy it! Thanks for explaining. :)

@joshtriplett
Copy link
Member

File::create returns a file in write-only mode but File::create_new returns a file in read-write mode. What is the reason for this difference?

File::create is useful for "the user asked me to write to this specific filename, which may or may not exist", such as for a user-specified output file. In that scenario, if it does exist, it might be write-only (e.g. a magic device file). For instance, consider echo 1 | sudo tee /proc/sys/vm/drop_caches on Linux: that file can be written, but not read, and attempting to open it read-write will return "permission denied".

That doesn't apply to File::create_new: it always makes a new file, which means it will always be an ordinary file, so there's no reason it needs to be write-only. And given that it doesn't need to be write-only, read-write is the more convenient choice.

@m-ou-se
Copy link
Member

m-ou-se commented Nov 23, 2023

Thanks for those very clear explanations, @joshtriplett!

@m-ou-se m-ou-se removed the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Dec 5, 2023
@m-ou-se
Copy link
Member

m-ou-se commented Dec 5, 2023

(This is now just waiting on a stabilization PR.)

@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Dec 14, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Dec 23, 2023
@bors bors closed this as completed in 62d5321 Dec 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants