Hi,
I created the following: https://pharo.manuscript.com/f/cases/21858/Cleanup-remaining-DeprecatedFileSystem-users This is an umbrella issue, please create sub issues for each set of changes. It is better to work step by step. The package DeprecatedFileSystem contains classes that should no longer be used, the most important being FileStream, StandardFileStream and MultiByteFileStream MultiByteBinaryOrTextStream and RWBinaryOrTextStream Alternatives exist by using FileSystem's FileReference as well as File and various new (Zn) streams. Let's all work together to make this real. It is important that the image itself makes correct use of ongoing refactoring efforts. We already took great steps, but there is more work to do. Note that in many cases, using code can and should be simplified, removing ugly hacks where possible. Feel free to discuss any issues on the ML. Sven |
Hi Sven,
in that process, how does one rewrite StandardFilestream subclasses? Regards, Thierry 2018-05-09 11:51 GMT+02:00 Sven Van Caekenberghe <[hidden email]>: > Hi, > > I created the following: > > > https://pharo.manuscript.com/f/cases/21858/Cleanup-remaining-DeprecatedFileSystem-users > > This is an umbrella issue, please create sub issues for each set of changes. It is better to work step by step. > > The package DeprecatedFileSystem contains classes that should no longer be used, the most important being > > FileStream, StandardFileStream and MultiByteFileStream > MultiByteBinaryOrTextStream and RWBinaryOrTextStream > > Alternatives exist by using FileSystem's FileReference as well as File and various new (Zn) streams. > > > Let's all work together to make this real. It is important that the image itself makes correct use of ongoing refactoring efforts. We already took great steps, but there is more work to do. Note that in many cases, using code can and should be simplified, removing ugly hacks where possible. > > Feel free to discuss any issues on the ML. > > Sven |
You mean you made a subclass of StandardFilestream yourself ?
If yes, can you describe briefly why you did so ? > On 9 May 2018, at 13:12, Thierry Goubier <[hidden email]> wrote: > > Hi Sven, > > in that process, how does one rewrite StandardFilestream subclasses? > > Regards, > > Thierry > > 2018-05-09 11:51 GMT+02:00 Sven Van Caekenberghe <[hidden email]>: >> Hi, >> >> I created the following: >> >> >> https://pharo.manuscript.com/f/cases/21858/Cleanup-remaining-DeprecatedFileSystem-users >> >> This is an umbrella issue, please create sub issues for each set of changes. It is better to work step by step. >> >> The package DeprecatedFileSystem contains classes that should no longer be used, the most important being >> >> FileStream, StandardFileStream and MultiByteFileStream >> MultiByteBinaryOrTextStream and RWBinaryOrTextStream >> >> Alternatives exist by using FileSystem's FileReference as well as File and various new (Zn) streams. >> >> >> Let's all work together to make this real. It is important that the image itself makes correct use of ongoing refactoring efforts. We already took great steps, but there is more work to do. Note that in many cases, using code can and should be simplified, removing ugly hacks where possible. >> >> Feel free to discuss any issues on the ML. >> >> Sven > |
2018-05-09 13:14 GMT+02:00 Sven Van Caekenberghe <[hidden email]>:
> You mean you made a subclass of StandardFilestream yourself ? There are a few subclasses of StandardFileStream in OSProcess: AttachableFileStream, AsyncFileReadStream and BufferedAsyncFileReadStream. They will need to be ported. > If yes, can you describe briefly why you did so ? As far as my analysis go, AttachableFileStream refers to and manipulates the underlying ioHandle of a unix pipe stream, AsyncFileReadStream interacts with the asynchronous io extensions of the vm to observe that io handle, and a BufferedAsyncFileReadStream add a thread-safe, buffered, blocking API for smalltalk processe on top. Regards, Thierry >> On 9 May 2018, at 13:12, Thierry Goubier <[hidden email]> wrote: >> >> Hi Sven, >> >> in that process, how does one rewrite StandardFilestream subclasses? >> >> Regards, >> >> Thierry >> >> 2018-05-09 11:51 GMT+02:00 Sven Van Caekenberghe <[hidden email]>: >>> Hi, >>> >>> I created the following: >>> >>> >>> https://pharo.manuscript.com/f/cases/21858/Cleanup-remaining-DeprecatedFileSystem-users >>> >>> This is an umbrella issue, please create sub issues for each set of changes. It is better to work step by step. >>> >>> The package DeprecatedFileSystem contains classes that should no longer be used, the most important being >>> >>> FileStream, StandardFileStream and MultiByteFileStream >>> MultiByteBinaryOrTextStream and RWBinaryOrTextStream >>> >>> Alternatives exist by using FileSystem's FileReference as well as File and various new (Zn) streams. >>> >>> >>> Let's all work together to make this real. It is important that the image itself makes correct use of ongoing refactoring efforts. We already took great steps, but there is more work to do. Note that in many cases, using code can and should be simplified, removing ugly hacks where possible. >>> >>> Feel free to discuss any issues on the ML. >>> >>> Sven >> > > |
> On 9 May 2018, at 13:41, Thierry Goubier <[hidden email]> wrote: > > 2018-05-09 13:14 GMT+02:00 Sven Van Caekenberghe <[hidden email]>: >> You mean you made a subclass of StandardFilestream yourself ? > > There are a few subclasses of StandardFileStream in OSProcess: > AttachableFileStream, AsyncFileReadStream and > BufferedAsyncFileReadStream. They will need to be ported. > >> If yes, can you describe briefly why you did so ? > > As far as my analysis go, AttachableFileStream refers to and > manipulates the underlying ioHandle of a unix pipe stream, > AsyncFileReadStream interacts with the asynchronous io extensions of > the vm to observe that io handle, and a BufferedAsyncFileReadStream > add a thread-safe, buffered, blocking API for smalltalk processe on > top. Yes, they will have to be ported. If it were me (but I haven't looked at the code) I would not inherit from those classes, but from a simpler Stream class, possibly even none. The stream API is actually quite small (or it should be). Inheritance is an implementation technique, inheriting from a complex class can be hard, unless the superclass was really designed with that purpose. All this, IMHO. > Regards, > > Thierry > >>> On 9 May 2018, at 13:12, Thierry Goubier <[hidden email]> wrote: >>> >>> Hi Sven, >>> >>> in that process, how does one rewrite StandardFilestream subclasses? >>> >>> Regards, >>> >>> Thierry >>> >>> 2018-05-09 11:51 GMT+02:00 Sven Van Caekenberghe <[hidden email]>: >>>> Hi, >>>> >>>> I created the following: >>>> >>>> >>>> https://pharo.manuscript.com/f/cases/21858/Cleanup-remaining-DeprecatedFileSystem-users >>>> >>>> This is an umbrella issue, please create sub issues for each set of changes. It is better to work step by step. >>>> >>>> The package DeprecatedFileSystem contains classes that should no longer be used, the most important being >>>> >>>> FileStream, StandardFileStream and MultiByteFileStream >>>> MultiByteBinaryOrTextStream and RWBinaryOrTextStream >>>> >>>> Alternatives exist by using FileSystem's FileReference as well as File and various new (Zn) streams. >>>> >>>> >>>> Let's all work together to make this real. It is important that the image itself makes correct use of ongoing refactoring efforts. We already took great steps, but there is more work to do. Note that in many cases, using code can and should be simplified, removing ugly hacks where possible. >>>> >>>> Feel free to discuss any issues on the ML. >>>> >>>> Sven >>> >> >> > |
On Wed., 9 May 2018, 13:49 Sven Van Caekenberghe, <[hidden email]> wrote:
I agree that these classes will need to be ported, just a little food for thought: I believe OSProcess is currently common between Squeak and Pharo - this porting will require that OSProcess is forked for Pharo, so the two implementations will start to diverge unless someone actively keeps them in sync. (Please don't read this as a negative judgement, it is just a statement of my understanding of the situation). There are a number of plugin primitives that Dave added to OSProcess because FilePlugin didn't meet his requirements. Some of the recent changes made to FilePlugin reduce the need for additional primitives (it is possible to connect to an existing file or file descriptor, #atEnd works for pipes), and I think there are additional enhancements that can be made which would reduce the number even further (mark files as non-blocking) (but I haven't looked in to it properly and would want to get Dave's input first). Cheers, Alistair |
> On 9 May 2018, at 16:06, Alistair Grant <[hidden email]> wrote: > > > > On Wed., 9 May 2018, 13:49 Sven Van Caekenberghe, <[hidden email]> wrote: > > > > On 9 May 2018, at 13:41, Thierry Goubier <[hidden email]> wrote: > > > > 2018-05-09 13:14 GMT+02:00 Sven Van Caekenberghe <[hidden email]>: > >> You mean you made a subclass of StandardFilestream yourself ? > > > > There are a few subclasses of StandardFileStream in OSProcess: > > AttachableFileStream, AsyncFileReadStream and > > BufferedAsyncFileReadStream. They will need to be ported. > > > >> If yes, can you describe briefly why you did so ? > > > > As far as my analysis go, AttachableFileStream refers to and > > manipulates the underlying ioHandle of a unix pipe stream, > > AsyncFileReadStream interacts with the asynchronous io extensions of > > the vm to observe that io handle, and a BufferedAsyncFileReadStream > > add a thread-safe, buffered, blocking API for smalltalk processe on > > top. > > Yes, they will have to be ported. > > If it were me (but I haven't looked at the code) I would not inherit from those classes, but from a simpler Stream class, possibly even none. The stream API is actually quite small (or it should be). > > Inheritance is an implementation technique, inheriting from a complex class can be hard, unless the superclass was really designed with that purpose. > > All this, IMHO. > > > I agree that these classes will need to be ported, just a little food > for thought: > > I believe OSProcess is currently common between Squeak and Pharo - this > porting will require that OSProcess is forked for Pharo, so the two > implementations will start to diverge unless someone actively keeps them > in sync. (Please don't read this as a negative judgement, it is > just a statement of my understanding of the situation). > > > There are a number of plugin primitives that Dave added to OSProcess > because FilePlugin didn't meet his requirements. Some of the recent > changes made to FilePlugin reduce the need for additional primitives (it > is possible to connect to an existing file or file descriptor, #atEnd > works for pipes), and I think there are additional enhancements that can > be made which would reduce the number even further (mark files as > non-blocking) (but I haven't looked in to it properly and would want to > get Dave's input first). > > > Cheers, > Alistair Of course we should keep the good stuff and help in maintaining OSProcess. That will probably mean that OSProcess will need dialect specific sub packages for some of its implementation. And Alistair, you did a lot of good work in this area already, we/I need you to help along. Sven |
In reply to this post by alistairgrant
Hi Alistair,
2018-05-09 16:06 GMT+02:00 Alistair Grant <[hidden email]>: > > > On Wed., 9 May 2018, 13:49 Sven Van Caekenberghe, <[hidden email]> wrote: >> >> >> >> > On 9 May 2018, at 13:41, Thierry Goubier <[hidden email]> >> > wrote: >> > >> > 2018-05-09 13:14 GMT+02:00 Sven Van Caekenberghe <[hidden email]>: >> >> You mean you made a subclass of StandardFilestream yourself ? >> > >> > There are a few subclasses of StandardFileStream in OSProcess: >> > AttachableFileStream, AsyncFileReadStream and >> > BufferedAsyncFileReadStream. They will need to be ported. >> > >> >> If yes, can you describe briefly why you did so ? >> > >> > As far as my analysis go, AttachableFileStream refers to and >> > manipulates the underlying ioHandle of a unix pipe stream, >> > AsyncFileReadStream interacts with the asynchronous io extensions of >> > the vm to observe that io handle, and a BufferedAsyncFileReadStream >> > add a thread-safe, buffered, blocking API for smalltalk processe on >> > top. >> >> Yes, they will have to be ported. >> >> If it were me (but I haven't looked at the code) I would not inherit from >> those classes, but from a simpler Stream class, possibly even none. The >> stream API is actually quite small (or it should be). >> >> Inheritance is an implementation technique, inheriting from a complex >> class can be hard, unless the superclass was really designed with that >> purpose. >> >> All this, IMHO. > > > > I agree that these classes will need to be ported, just a little food > for thought: > > I believe OSProcess is currently common between Squeak and Pharo - this > porting will require that OSProcess is forked for Pharo, so the two > implementations will start to diverge unless someone actively keeps them > in sync. (Please don't read this as a negative judgement, it is > just a statement of my understanding of the situation). I agree with your statement. Since I need OSProcess, I will probably undertake this, even if I don't see exactly how this would be done in the context of the development process of OSProcess today. The easiest would be to have a compatibility package loaded for OSProcess on Pharo > 70XXX; the hard part will be whether that XXX will be clearly known. > There are a number of plugin primitives that Dave added to OSProcess > because FilePlugin didn't meet his requirements. Some of the recent > changes made to FilePlugin reduce the need for additional primitives (it > is possible to connect to an existing file or file descriptor, #atEnd > works for pipes), and I think there are additional enhancements that can > be made which would reduce the number even further (mark files as > non-blocking) (but I haven't looked in to it properly and would want to > get Dave's input first). Agreed, and, yes, I've been loosely following what you were doing in this area. Anything that can improve Pharo robustness in that area will be appreciated, I had far too many lockups in the past to be happy with it. I'm a bit worried about VM changes now; I've played a bit too much with downloading my own VM from bintray for compensate for stability issues of the blessed VMs, so I'm not sure to be able to track custom OSProcess configurations depending on the presence of specific primitives in the VM... sort of like running a configure script for compiling a C library. Regards, Thierry > > Cheers, > Alistair > |
In reply to this post by Sven Van Caekenberghe-2
Hi Sven,
On 9 May 2018 at 16:11, Sven Van Caekenberghe <[hidden email]> wrote: > > ... > > Of course we should keep the good stuff and help in maintaining OSProcess. > That will probably mean that OSProcess will need dialect specific sub packages for some of its implementation. > > And Alistair, you did a lot of good work in this area already, we/I need you to help along. Thanks very much for your kind words (again). Hopefully I can continue to contribute and help improve Pharo, including all the great work you've done. Thanks again, Alistair |
In reply to this post by Thierry Goubier
On Wed, May 09, 2018 at 01:41:47PM +0200, Thierry Goubier wrote:
> 2018-05-09 13:14 GMT+02:00 Sven Van Caekenberghe <[hidden email]>: > > You mean you made a subclass of StandardFilestream yourself ? > > There are a few subclasses of StandardFileStream in OSProcess: > AttachableFileStream, AsyncFileReadStream and > BufferedAsyncFileReadStream. They will need to be ported. > > > If yes, can you describe briefly why you did so ? > > As far as my analysis go, AttachableFileStream refers to and > manipulates the underlying ioHandle of a unix pipe stream, > AsyncFileReadStream interacts with the asynchronous io extensions of > the vm to observe that io handle, and a BufferedAsyncFileReadStream > add a thread-safe, buffered, blocking API for smalltalk processe on > top. That's right. I'm sorry I have not been keeping up with this discussion, but one thought that comes to mind - the AttachableFileStream hierarchy is something that would better be replaced by some kind of delegation. It was a hack that I did a long time ago to allow OSProcess to work without modifying the base image. But since then, we have added multibyte characters and streams, and we have different file and stream models in Squeak/Pharo/Cuis (Cuis is an important target platform from my POV). I don't have a solution in mind, my sense is that if we find a way to make AttachableFileStream go away, then that solution would very likely make it unnecessary to develop and maintain separate Squeak/Pharo/Cuis forks moving forward. Note that the only fundamental thing that AttachableFileStream does is this: AttachableFileStream class>>name: aSymbolOrString attachTo: anIOHandle writable: readWriteFlag "Create a new instance attached to anIOHandle, where anIOHandle represents an open IO channel. For write streams, this represents two Smalltalk streams which write to the same OS file or output stream, presumably with interleaved output. The purpose of this method is to permit a FileStream to be attached to an existing IOHandle, such as the IOHandle for standard input, standard output, and standard error." ^ (super basicNew name: aSymbolOrString attachTo: anIOHandle writable: readWriteFlag) initialize The method comment refers to "IOHandle" which was another old project I was doing, but IOHandle basically means the #fileID field of a file stream. If it was just AttachableFileStream that needed to be refactored out of existence, it would be easy. But I later added quite a bit a behavior to BufferedAsyncFileReadStream and I'm not sure what might be needed to make this work properly. Sven, I suspect that you are the most knowledgeable in this area, so any suggestions would be welcome. Apologies once again for not keeping up with this, but I am glad to see the discussion. Dave |
Hi Dave,
On 11 May 2018 at 03:58, David T. Lewis <[hidden email]> wrote: > On Wed, May 09, 2018 at 01:41:47PM +0200, Thierry Goubier wrote: >> 2018-05-09 13:14 GMT+02:00 Sven Van Caekenberghe <[hidden email]>: >> > You mean you made a subclass of StandardFilestream yourself ? >> >> There are a few subclasses of StandardFileStream in OSProcess: >> AttachableFileStream, AsyncFileReadStream and >> BufferedAsyncFileReadStream. They will need to be ported. >> >> > If yes, can you describe briefly why you did so ? >> >> As far as my analysis go, AttachableFileStream refers to and >> manipulates the underlying ioHandle of a unix pipe stream, >> AsyncFileReadStream interacts with the asynchronous io extensions of >> the vm to observe that io handle, and a BufferedAsyncFileReadStream >> add a thread-safe, buffered, blocking API for smalltalk processe on >> top. > > That's right. > > I'm sorry I have not been keeping up with this discussion, but one thought > that comes to mind - the AttachableFileStream hierarchy is something that > would better be replaced by some kind of delegation. It was a hack that I > did a long time ago to allow OSProcess to work without modifying the base > image. But since then, we have added multibyte characters and streams, and > we have different file and stream models in Squeak/Pharo/Cuis (Cuis is an > important target platform from my POV). > > I don't have a solution in mind, my sense is that if we find a way to make > AttachableFileStream go away, then that solution would very likely make > it unnecessary to develop and maintain separate Squeak/Pharo/Cuis forks > moving forward. > > Note that the only fundamental thing that AttachableFileStream does is this: > > AttachableFileStream class>>name: aSymbolOrString attachTo: anIOHandle writable: readWriteFlag > "Create a new instance attached to anIOHandle, where anIOHandle > represents an open IO channel. For write streams, this represents two > Smalltalk streams which write to the same OS file or output stream, > presumably with interleaved output. The purpose of this method is to > permit a FileStream to be attached to an existing IOHandle, such as > the IOHandle for standard input, standard output, and standard error." > > ^ (super basicNew > name: aSymbolOrString > attachTo: anIOHandle > writable: readWriteFlag) initialize > > > The method comment refers to "IOHandle" which was another old project I was > doing, but IOHandle basically means the #fileID field of a file stream. > > If it was just AttachableFileStream that needed to be refactored out of existence, > it would be easy. But I later added quite a bit a behavior to BufferedAsyncFileReadStream > and I'm not sure what might be needed to make this work properly. > > Sven, I suspect that you are the most knowledgeable in this area, so any > suggestions would be welcome. Are the two primitives that were added to the VM a couple of months ago (primitiveConnectToFileDescriptor & primitiveConnectToFile) enough to allow AttachableFileStream to be removed? Also, borrowing primitiveSQFileSetNonBlocking from OSProcess I was able to open a named pipe and read from it in a non-blocking way using the standard Pharo 7 file stream (BinaryFileStream). I'd like to look at getting AsyncFile working with it, but that's a long way down the ToDo list. HTH, Alistair |
Hi Alistair,
On Fri, May 11, 2018 at 03:09:20PM +0200, Alistair Grant wrote: > Hi Dave, > > On 11 May 2018 at 03:58, David T. Lewis <[hidden email]> wrote: > > On Wed, May 09, 2018 at 01:41:47PM +0200, Thierry Goubier wrote: > >> 2018-05-09 13:14 GMT+02:00 Sven Van Caekenberghe <[hidden email]>: > >> > You mean you made a subclass of StandardFilestream yourself ? > >> > >> There are a few subclasses of StandardFileStream in OSProcess: > >> AttachableFileStream, AsyncFileReadStream and > >> BufferedAsyncFileReadStream. They will need to be ported. > >> > >> > If yes, can you describe briefly why you did so ? > >> > >> As far as my analysis go, AttachableFileStream refers to and > >> manipulates the underlying ioHandle of a unix pipe stream, > >> AsyncFileReadStream interacts with the asynchronous io extensions of > >> the vm to observe that io handle, and a BufferedAsyncFileReadStream > >> add a thread-safe, buffered, blocking API for smalltalk processe on > >> top. > > > > That's right. > > > > I'm sorry I have not been keeping up with this discussion, but one thought > > that comes to mind - the AttachableFileStream hierarchy is something that > > would better be replaced by some kind of delegation. It was a hack that I > > did a long time ago to allow OSProcess to work without modifying the base > > image. But since then, we have added multibyte characters and streams, and > > we have different file and stream models in Squeak/Pharo/Cuis (Cuis is an > > important target platform from my POV). > > > > I don't have a solution in mind, my sense is that if we find a way to make > > AttachableFileStream go away, then that solution would very likely make > > it unnecessary to develop and maintain separate Squeak/Pharo/Cuis forks > > moving forward. > > > > Note that the only fundamental thing that AttachableFileStream does is this: > > > > AttachableFileStream class>>name: aSymbolOrString attachTo: anIOHandle writable: readWriteFlag > > "Create a new instance attached to anIOHandle, where anIOHandle > > represents an open IO channel. For write streams, this represents two > > Smalltalk streams which write to the same OS file or output stream, > > presumably with interleaved output. The purpose of this method is to > > permit a FileStream to be attached to an existing IOHandle, such as > > the IOHandle for standard input, standard output, and standard error." > > > > ^ (super basicNew > > name: aSymbolOrString > > attachTo: anIOHandle > > writable: readWriteFlag) initialize > > > > > > The method comment refers to "IOHandle" which was another old project I was > > doing, but IOHandle basically means the #fileID field of a file stream. > > > > If it was just AttachableFileStream that needed to be refactored out of existence, > > it would be easy. But I later added quite a bit a behavior to BufferedAsyncFileReadStream > > and I'm not sure what might be needed to make this work properly. > > > > Sven, I suspect that you are the most knowledgeable in this area, so any > > suggestions would be welcome. > > Are the two primitives that were added to the VM a couple of months > ago (primitiveConnectToFileDescriptor & primitiveConnectToFile) enough > to allow AttachableFileStream to be removed? No, I was referring only to the actual file stream hierarchy. Long ago, I tapped into StandardFileStream with some subclasses that do useful things for OSProcess. That is clearly a hack, and I'm sure we could do better. Having said that, I have not looked at Pharo 7 yet so maybe I don't know what I'm talking about ;-) > > Also, borrowing primitiveSQFileSetNonBlocking from OSProcess I was > able to open a named pipe and read from it in a non-blocking way using > the standard Pharo 7 file stream (BinaryFileStream). I'd like to look > at getting AsyncFile working with it, but that's a long way down the > ToDo list. Good, primitiveSQFileSetNonBlocking certainly should work for that, and I'm glad to hear that it does. Dave > > HTH, > Alistair |
Hi Dave,
On 12 May 2018 at 01:28, David T. Lewis <[hidden email]> wrote: > Hi Alistair, > > On Fri, May 11, 2018 at 03:09:20PM +0200, Alistair Grant wrote: >> Hi Dave, >> >> ... >> >> Are the two primitives that were added to the VM a couple of months >> ago (primitiveConnectToFileDescriptor & primitiveConnectToFile) enough >> to allow AttachableFileStream to be removed? > > No, I was referring only to the actual file stream hierarchy. Long ago, > I tapped into StandardFileStream with some subclasses that do useful things > for OSProcess. That is clearly a hack, and I'm sure we could do better. > Having said that, I have not looked at Pharo 7 yet so maybe I don't know > what I'm talking about ;-) The two primitives aren't Pharo specific - although I don't think the primitive methods have been added to Squeak yet: primitiveConnectToFileDescriptor takes a posix file descriptor (integer) and returns a file handle (SQFile byte array). primitiveConnectToFile takes a posix FILE* pointer (byte array size FILE*) and returns a file handle (SQFile byte array). I suspect that AttachableFileStream does something else since it looks like #name:attachTo:writable: takes a SQFile byte array (I have to admit to not looking very carefully). Cheers, Alistair |
Free forum by Nabble | Edit this page |