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

stdlib Suggestion: Open File for Appending #14375

Open
jcalabro opened this issue Jan 19, 2023 · 14 comments
Open

stdlib Suggestion: Open File for Appending #14375

jcalabro opened this issue Jan 19, 2023 · 14 comments
Labels
enhancement Solving this issue will likely involve adding new logic or components to the codebase. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. standard library This issue involves writing Zig code for the standard library.
Milestone

Comments

@jcalabro
Copy link
Sponsor Contributor

Hey there! Apologies if this has been discussed before; I did a search through Zig's issues but wasn't able to find anything on the topic.

I'd like to be able to call std.fs.openFileAbsolute (and similar functions) and by some mechanism supply O_APPEND or FILE_APPEND_DATA. For instance, I've got a log file on disk that I want to append to during restarts of my program; I don't want to truncate the file and start from scratch every time (truncating is the current behavior).

Currently, I think the only way to do this in the fs package. I can call os.open directly with these flags, but then I just get back an os.fd_t rather than a fs.File, which is not quite as nice to use. I'm pretty new to Zig, so apologies if there is a way to easily achieve what I want and I'm missing it.

Assuming that's not the case, I think there may be a minimally invasive way to get this done, though I'm definitely open to feedback: I propose adding a new enum field to fs.File.OpenFlags called TruncateMode or similar that has:

pub const TruncateMode = enum {
    truncate, // default
    append,
};

If we open a file with write capabilites (write_only or read_write) and append, then we'd also want to OR that with O.APPEND in fs.openFileZ (and its windows counterpart).

Thoughts? If this seems like a good approach, I'm happy to take a shot at implementing it. Thanks!

@Vexu
Copy link
Member

Vexu commented Jan 19, 2023

fs.createFile does take fs.File.CreateFlags with truncate but setting it to false is currently a no-op so it defaults to truncating either way.

@Vexu Vexu added bug Observed behavior contradicts documented or intended behavior enhancement Solving this issue will likely involve adding new logic or components to the codebase. contributor friendly This issue is limited in scope and/or knowledge of Zig internals. standard library This issue involves writing Zig code for the standard library. labels Jan 19, 2023
@Vexu Vexu added this to the 0.11.0 milestone Jan 19, 2023
@jcalabro
Copy link
Sponsor Contributor Author

Ah good point, so perhaps adding a truncate: bool to fs.File.OpenFlags would be a better approach than an enum for consistency?

@Vexu
Copy link
Member

Vexu commented Jan 19, 2023

.truncate = false should be fixed to not truncate the file.

@jcalabro
Copy link
Sponsor Contributor Author

I do see the issue on createFile, and I suppose I could repeatedly call that to get a File back, but do you think it is valuable to also add this behavior to the openFile calls?

I can submit a quick patch to fix the bug you mentioned, looks like a very quick fix.

@Vexu
Copy link
Member

Vexu commented Jan 19, 2023

I don't have too strong of an opinion about it, I guess there could be value to asserting a file exists before appending to it?

@nektro
Copy link
Contributor

nektro commented Jan 19, 2023

var file = try std.fs.cwd().openFile(path, .{ .mode = .read_write });
var stat = try file.stat();
try file.seekTo(stat.size);

try file.writer().writeAll(); // this will happen at the end of the file

@jcalabro
Copy link
Sponsor Contributor Author

@nektro thanks for that example! It definitely does what I want.

The suggestion of this issue is that, in addition, since O_APPEND exists in the Linux open system call, it would be a nice feature in the zig std library to also have an append option on std.fs.File.OpenFlags.

If there's agreement that it would be a valuable addition, I'd be happy to take a shot at adding it! 😄

@jcalabro
Copy link
Sponsor Contributor Author

Hi again @Vexu! I realized while implementing a test case for #14376 that we should be careful here.

I think this statement is incorrect: .truncate = false should be fixed to not truncate the file. Currently, .truncate = false does not truncate the file; it just opens a fp at the start of the file, and you're able to write over the data at the start of the file from there (or seek to a location and keep writing). So it's not truncating, it's just opening the file and pointing at the first byte. I think this is the intended behavior?

More generally though, I don't want to touch createFile. I'm not quite sure what O_APPEND would even mean in the context of createFile 😄

Instead, I still wish to complete the work I outlined in the original post of this issue. I want to add the ability to open a file in "truncate mode", "append mode", or "start of file" mode. Basically, I want more similarity to what is supplied by the open system call.

@praschke
Copy link
Contributor

@nektro Unix filesystem attributes/flags (including Mac) and NT, POSIX, and NFSv4 ACLs all have specific append permissions, where opening in write mode can fail.

@kuon
Copy link
Sponsor Contributor

kuon commented Feb 8, 2023

I agree with @jcalabro in what we need to have separate truncate or append.

Maybe we can take some inspiration from the rust approach? https://doc.rust-lang.org/std/fs/struct.OpenOptions.html

Basically, the rust fs open options has the following booleans:

read
write
append
truncate
create
create_new

Also, as @praschke said, it is possible to have only append permission on a file, (without read and write), which needs to be exposed because it is a common use for writing to log files. With the above, it would be append = true with all the other flags to false.

@kuon
Copy link
Sponsor Contributor

kuon commented Feb 8, 2023

While speaking of this, there is also an append permission in NFSv4 ACL for directories that give permission to only create sub-directories and not files. This is not directly related to this issue, but I thought I'd mention it.

@andrewrk andrewrk added proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. and removed bug Observed behavior contradicts documented or intended behavior contributor friendly This issue is limited in scope and/or knowledge of Zig internals. labels Jul 20, 2023
@andrewrk andrewrk modified the milestones: 0.11.0, 0.14.0 Jul 20, 2023
@andrewrk
Copy link
Member

andrewrk commented Jul 20, 2023

I'm not convinced that the standard library cross-platform abstractions should support the use case of append mode, and so I have changed this from bug to proposal and deprioritized it. Note that I still expect std.posix (#5019) to have this functionality.

@karlseguin
Copy link
Contributor

Until this is decided, can the truncate field of File.CreateFlags be removed if it's being ignored?

@Calder-Ty
Copy link

var file = try std.fs.cwd().openFile(path, .{ .mode = .read_write });
var stat = try file.stat();
try file.seekTo(stat.size);

try file.writer().writeAll(); // this will happen at the end of the file

I want to note that this solution is susceptible to race conditions. If you want to avoid that you will have to go to they underlying system calls, as I don't see a way to atomically append to a file using the openFile API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Solving this issue will likely involve adding new logic or components to the codebase. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

No branches or pull requests

8 participants