[Seaside-dev] Grease on Pharo7: streams
Johan Brichau
johan at inceptive.be
Tue Jul 24 08:18:39 UTC 2018
So, after digging around, I have corrected my report on the broken ReadWriteStream behavior.
The problem is described here: https://pharo.fogbugz.com/f/cases/22281/ReadWriteStream-broken <https://pharo.fogbugz.com/f/cases/22281/ReadWriteStream-broken>
I’ll make a PR for Pharo and let’s see where it goes.
Johan
> On 23 Jul 2018, at 21:03, Johan Brichau <johan at inceptive.be> wrote:
>
> Hi Paul,
>
> I’ve been somewhat afloat in my mind trying to figure out what was wrong.
> Unfortunately, I think that `(ReadWriteStream on: (String new: 4096)) contents` returning a string with null chars was due to my own changes trying to fix the tests.
>
> My latest changes make fixes to Pharo to get back to the desired behavior. (https://github.com/SeasideSt/Grease/commits/pharo7 <https://github.com/SeasideSt/Grease/commits/pharo7>)
>
> They actually revert 2 changes in Pharo 7 and I do not see why they were done.
> There are also very few tests for streams in Pharo itself. I’m trying to make a PR to fix that.
>
> I must correct in that (ReadWriteStream on: 'abc') contents should indeed return an empty string, as pointed out by Richard on the mailinglist in http://lists.pharo.org/pipermail/pharo-users_lists.pharo.org/2018-July/040162.html <http://lists.pharo.org/pipermail/pharo-users_lists.pharo.org/2018-July/040162.html>
>
> In this respect, I discovered another change that introduced the ReadWriteStream>>on: method which, strangely, sets the readLimit position to the end of the stream. Removing this method (introduced in https://github.com/pharo-project/pharo/commit/41f3a88863f7d9b35509e6e43f7d3ac2df9801e4#diff-ba7cc7962a4a22468a18b31fe75ada04) and reverting the change in https://github.com/pharo-project/pharo/commit/9e442addff3790af0c78dbc73f1f349de8a4ad03#diff-ba7cc7962a4a22468a18b31fe75ada04 re-establishes the stream behavior that is tested by the stream tests in Grease.
>
> I hope I’m on the right path this time ;)
>
> cheers
> Johan
>
>> On 23 Jul 2018, at 20:33, Paul DeBruicker <pdebruic at gmail.com <mailto:pdebruic at gmail.com>> wrote:
>>
>>
>> Hi Johan,
>>
>>
>> I just downloaded Pharo 7 (on the mac, 64bit image/vm) to try this out
>> because I was wondering if it was filling with spaces or null characters and
>> inspecting
>>
>>
>> (ReadWriteStream on: (String new: 4096)) contents
>>
>>
>> gave an empty string. Could something else cause the 4096 char string?
>>
>>
>>
>>
>>
>>
>> Johan Brichau-2 wrote
>>> Hi all,
>>>
>>> There have been a number of changes to the ReadWriteStream implementation
>>> in Pharo 7 that break Grease stream tests:
>>> https://travis-ci.org/SeasideSt/Grease/jobs/406766720 <https://travis-ci.org/SeasideSt/Grease/jobs/406766720>
>>> <https://travis-ci.org/SeasideSt/Grease/jobs/406766720> <https://travis-ci.org/SeasideSt/Grease/jobs/406766720>>;
>>> I think there is both an issue in Grease for Pharo and in Pharo 7 itself.
>>>
>>> * The most important issue is that the following expression returns a
>>> different result on Pharo 7 when compared with earlier versions:
>>>
>>> (ReadWriteStream on: (String new: 4096)) contents
>>>
>>> On Pharo 6, this returns an empty string.
>>> On Pharo 7, this returns a string with 4096 white space chars.
>>>
>>> I’m inclined to say that Pharo 7 behavior is correct.
>>> I read through the ANSI document and `contents` should return the future
>>> sequences as well… which effectively means returning all those empty
>>> chars.
>>> I’m a bit in doubt why this has been different all that time… hence my
>>> email.
>>>
>>> The core of the issue is that GRPharoPlatform>>readWriteCharacterStream is
>>> `ReadWriteStream on: (String new: 4096)`, hence a String instance with a
>>> lot of spaces ;)
>>> In GRGemStonePlatform, the readWriteStream is initialized on an empty
>>> string.
>>> In GRSqueakPlatform, the RWBinaryOrTextStream class is used on ‘ByteArray
>>> new: 4096’.
>>>
>>> Changing the implementation of GRPharoPlatform>>readWriteCharacterStream
>>> to `ReadWriteStream on: String new` fixes most of the tests.
>>>
>>> * The next issue is that `ReadWriteStream>>contents` has been changed in
>>> https://github.com/pharo-project/pharo/commit/9e442addff3790af0c78dbc73f1f349de8a4ad03#diff-ba7cc7962a4a22468a18b31fe75ada04 <https://github.com/pharo-project/pharo/commit/9e442addff3790af0c78dbc73f1f349de8a4ad03#diff-ba7cc7962a4a22468a18b31fe75ada04>
>>> <https://github.com/pharo-project/pharo/commit/9e442addff3790af0c78dbc73f1f349de8a4ad03#diff-ba7cc7962a4a22468a18b31fe75ada04> <https://github.com/pharo-project/pharo/commit/9e442addff3790af0c78dbc73f1f349de8a4ad03#diff-ba7cc7962a4a22468a18b31fe75ada04>>;
>>> This means that:
>>>
>>> | stream |
>>> stream := GRPlatform current readWriteCharacterStream.
>>> stream
>>> nextPutAll: 'abc';
>>> reset.
>>> self assert: stream contents = 'abc’.
>>>
>>> This is no longer true in Pharo 7.
>>> ANSI docs says that `reset` should only reset the pointer position and
>>> thus the future sequences on the stream should still be returned after
>>> resetting.
>>> Reverting the change in the commit, also fixes that test.
>>> It’s unclear why that change was made though, so I’ll try to talk to
>>> someone at Pharo.
>>>
>>> Would be nice to hear other’s thoughts.
>>>
>>> Cheers
>>> Johan
>>> _______________________________________________
>>> seaside-dev mailing list
>>
>>> seaside-dev at .squeakfoundation
>>
>>> http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev <http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev>
>>
>>
>>
>>
>>
>> --
>> Sent from: http://forum.world.st/Seaside-Development-f1294793.html <http://forum.world.st/Seaside-Development-f1294793.html>
>> _______________________________________________
>> seaside-dev mailing list
>> seaside-dev at lists.squeakfoundation.org <mailto:seaside-dev at lists.squeakfoundation.org>
>> http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev <http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/seaside-dev/attachments/20180724/45559d9b/attachment-0001.html>
More information about the seaside-dev
mailing list