BinaryFileStream should not truncate on flush

[kurtkilpela]

In f219c5e, BinaryFileStream>>#flush was updated to truncate the file to the current position. This was implemented as Rowan did not seem to truncate existing files when writing packages out to disk.

This should be reverted. If there is still an issue where package files aren't truncated, Rowan should be updated to properly truncate.

[dalehenrich]

out of curiosity ... what would be the proper way to truncate files?

[kurtkilpela]

Sending #truncate to the stream would be sufficient.

[dalehenrich]

So I would be expected to call truncate / close for every file on the off chance that it existed before and that it was larger before hand?

…

On Fri, Mar 4, 2022 at 9:27 AM Kurt Kilpela ***@***.***> wrote: Sending #truncate to the stream would be sufficient. — Reply to this email directly, view it on GitHub <#53 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAEYKT2TLXUCD3RMAY4O6Z3U6JBYHANCNFSM5P6AGD2A> . You are receiving this because you commented.Message ID: ***@***.***>

[dalehenrich]

... should #close do a truncate ... instead of flush?

[kurtkilpela]

I imagine the pattern would be stream flush; truncate; close. The problem with having truncate occur automatically, is that you may not actually want it. It is hard to work around if it happens automatically.

You could open a file stream, seek to a record in the middle of a file, write a few bytes to change the record, and then close it. If we automatically truncate the file, you'd always have to read the full file in, make the changes, and then write the whole file out. That doesn't scale well.

(Alternatively, in the Rowan case, you could truncate after opening the file.)

[martinmcclure]

In Pharo, sending truncate to a file stream appears to truncate the file to position 0; i.e. wipes the entire contents. So you wouldn't want to do that after writing contents you wanted to keep.

If you want to write an entire file, possibly creating it but not keeping any previous contents, the best way to do that is to specify the truncate option when opening the file. This results in the open() system call having the O_TRUNC flag set, and may not yet be supported by FileSystemGs (but should be at some point).
Alternatively, truncating after opening, as Kurt suggests, should work.

[kurtkilpela]

Yeah, I saw that behavior in Pharo. It wasn't yet clear to me that that was correct behavior. I suppose we will take the semantics since we are porting FileSystem. They do have a second selector #truncate: which allows you to truncate to any position.

The simplest thing would be to just truncate immediately after opening the file.

[dalehenrich]

ah, so an #openTruncate, in the same vein as #openAppend would be desired then? ... keep in mind that I don't know what the #openAppend pattern looks like either :)

…

On Fri, Mar 4, 2022 at 10:41 AM Kurt Kilpela ***@***.***> wrote: Yeah, I saw that behavior in Pharo. It wasn't yet clear to me that that was correct behavior. I suppose we will take the semantics since we are porting FileSystem. They do have a second selector #truncate: which allows you to truncate to any position. The simplest thing would be to just truncate immediately after opening the file. — Reply to this email directly, view it on GitHub <#53 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAEYKTZX6QBY7UBKH7PL3D3U6JKPPANCNFSM5P6AGD2A> . You are receiving this because you commented.Message ID: ***@***.***>

[kurtkilpela]

Pharo FileReference doesn't have an #openAppend. Pharo's File class does have an #openForAppend.

It would be possible for us to add several open methods. Or we could have an open that takes a FileOpeningOptions instance (which is the direction we've been heading.)

[martinmcclure]

The unary selector #truncate truncating to any position but zero would be quite unexpected behavior, so I think that Pharo's behavior is the expected one.

For opening options I think there should be #openFoo for only the most common options, for everything else use a FileOpeningOptions. Maybe something like myFileReference openWithOptions: FileOpeningOptions write truncate.

[kurtkilpela]

I find it a little weird that #truncate doesn't truncate to the current position for a Stream. I would agree that a send of #truncate to an instance of UnixFileDescriptor doing anything other than truncate to zero would be weird.

[dalehenrich]

Either openX or openWithOptions: would work for me ... seems that openX variants that call openWithOptions: for the common combos, would be useful ...

…

On Fri, Mar 4, 2022 at 11:03 AM martinmcclure ***@***.***> wrote: The unary selector #truncate truncating to any position but zero would be quite unexpected behavior, so I think that Pharo's behavior is the expected one. For opening options I think there should be #openFoo for only the most common options, for everything else use a FileOpeningOptions. Maybe something like myFileReference openWithOptions: FileOpeningOptions write truncate. — Reply to this email directly, view it on GitHub <#53 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAEYKT3IVXEVENHZSLIV273U6JM7FANCNFSM5P6AGD2A> . You are receiving this because you commented.Message ID: ***@***.***>

[dalehenrich]

Hmmm, if I am fiddling with the internal bits of an existing file, I would agree that truncate to current position would be one of things that might be useful to be able to do ... although I have to admit, I tend to rewrite an entire file when I want to change internal bits (truncate to 0) and don't have personal experience with fiddling with internals of a file, but it seems to be dicey business to be inserting and deleting bytes while keeping in mind the original end of file on disk so that you can make sure they are not left hanging around ... (truncate on flush would be a useful operation in this case, I suppose) On Fri, Mar 4, 2022 at 11:13 AM Dale Henrichs < ***@***.***> wrote:

…

Either openX or openWithOptions: would work for me ... seems that openX variants that call openWithOptions: for the common combos, would be useful ... On Fri, Mar 4, 2022 at 11:03 AM martinmcclure ***@***.***> wrote: > The unary selector #truncate truncating to any position but zero would be > quite unexpected behavior, so I think that Pharo's behavior is the expected > one. > > For opening options I think there should be #openFoo for only the most > common options, for everything else use a FileOpeningOptions. Maybe > something like myFileReference openWithOptions: FileOpeningOptions write > truncate. > > — > Reply to this email directly, view it on GitHub > <#53 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AAEYKT3IVXEVENHZSLIV273U6JM7FANCNFSM5P6AGD2A> > . > You are receiving this because you commented.Message ID: > ***@***.***> >

[martinmcclure]

It seems odd to me to have streams respond to truncate at all, it has no obvious meaning to me. Maybe truncateToHere as a shortcut for stream truncateTo: stream position, but there's the Pharo precedent to worry about.

[dalehenrich]

I suppose defaulting to Pharo behavior is not a bad way to go for truncate, while openWithOptions: (with short cuts) meets our needs and is an intentional departure from Pharo anyway ...

[kurtkilpela]

Matching the Pharo behavior is the best answer. Pharo does have a #truncate: which allows you to truncate to a specific length.

[kurtkilpela]

Added AbstractFileReference>>#truncatedWriteStreamDo:. Rowan's pattern is to use #writeStreamDo:. This new selector will truncate the file before evaluating the argument and should be a sufficient replacement for Rowan. See 0cac5ba.

Once Rowan has swapped to #truncatedWriteStreamDo:, #flush can stop truncating the file to the current position.

[kurtkilpela]

Note to self: sending #truncate to a stream, results in truncating the file to position 0, not the current position.

[kurtkilpela]

Per conversation with Dale, remove #truncatedWriteStreamDo: and #appendingWriteStreamDo:. Instead, the stream will just be created in appending mode. In the case of truncation, the file will be truncated as the first action in the #writeStreamDo: block argument.

[kurtkilpela]

844685f removes the previous send of #truncate in #flush. See #105

[kurtkilpela]

c0282f0 removes #truncatedWriteStreamDo: and '#appendingWriteStreamDo:`.

Rowan will send #truncate to the stream as necessary.

[kurtkilpela]

6fe072f ensures files are truncated to a length of zero bytes when an associated write stream receives #truncate.