P8 regression

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|

P8 regression

HilaireFernandes
Hi,

We have been hit in DrGeo by another regression in Pharo8 (it is not
present in DrGeo versions based on previous Pharo).

When loading a Sketch with accentuated Latin characters (same occurs
with Chinese but I have so far not enough information to assert it is
the same error) there is a ZnInvalidUTF8 error.

Attached Pharo8

See details https://bugs.launchpad.net/drgeo/+bug/1837745

For the record the stream passed to the XML parser (an old version but I
don't think it is the problem) is from readStream message send to
FileReference.

Do you want a bug ticket?

Hilaire

--
Dr. Geo
http://drgeo.eu


PharoDebug.log (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: P8 regression

Sven Van Caekenberghe-2
Can you include the file that you are trying to load ?

Also, the stack trace is very short.

Are you sure XML parser is ready for Pharo 8 (i.e. are its tests green) ?

> On 24 Jul 2019, at 16:59, Hilaire <[hidden email]> wrote:
>
> Hi,
>
> We have been hit in DrGeo by another regression in Pharo8 (it is not
> present in DrGeo versions based on previous Pharo).
>
> When loading a Sketch with accentuated Latin characters (same occurs
> with Chinese but I have so far not enough information to assert it is
> the same error) there is a ZnInvalidUTF8 error.
>
> Attached Pharo8
>
> See details https://bugs.launchpad.net/drgeo/+bug/1837745
>
> For the record the stream passed to the XML parser (an old version but I
> don't think it is the problem) is from readStream message send to
> FileReference.
>
> Do you want a bug ticket?
>
> Hilaire
>
> --
> Dr. Geo
> http://drgeo.eu
>
> <PharoDebug.log>


Reply | Threaded
Open this post in threaded view
|

Re: P7 regression

HilaireFernandes
Hi Sven,

Le 24/07/2019 à 17:05, Sven Van Caekenberghe a écrit :
> Can you include the file that you are trying to load ?

Attached.

It looks like issue come when saving the file. The UTF8 characters are
not saved properly. The unix file command was indicating it is an utf8
but Emacs shows it was different.

>
> Also, the stack trace is very short.

Yes, I saw that... I don't know why. I did not cut it. I got a longer
one this time. But it is likely not the cause.

> Are you sure XML parser is ready for Pharo 8 (i.e. are its tests green) ?

My bad, I am using P7. But I don't know either about the full
compatibility of the XMLParser version I am using, the one before
compatibility break.

It is now clear the is issue when saving. To write, I use WriteStream on
String. Should it be different?


Hilaire


--
Dr. Geo
http://drgeo.eu


test.fgeo (648 bytes) Download Attachment
PharoDebug.log (16K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: P7 regression

Sven Van Caekenberghe-2


> On 24 Jul 2019, at 17:32, Hilaire <[hidden email]> wrote:
>
> Hi Sven,
>
> Le 24/07/2019 à 17:05, Sven Van Caekenberghe a écrit :
>> Can you include the file that you are trying to load ?
>
> Attached.
>
> It looks like issue come when saving the file. The UTF8 characters are
> not saved properly. The unix file command was indicating it is an utf8
> but Emacs shows it was different.
>
>>
>> Also, the stack trace is very short.
>
> Yes, I saw that... I don't know why. I did not cut it. I got a longer
> one this time. But it is likely not the cause.
>
>> Are you sure XML parser is ready for Pharo 8 (i.e. are its tests green) ?
>
> My bad, I am using P7. But I don't know either about the full
> compatibility of the XMLParser version I am using, the one before
> compatibility break.
>
> It is now clear the is issue when saving. To write, I use WriteStream on
> String. Should it be different?

But how do you write it to a file then ?

FileLocator desktop / 'hilaire.xml' writeStreamDo: [ :out | out << 'Les élèves Françaises ont 10 €' ].

(FileLocator desktop / 'hilaire.xml') contents.

">> 'Les élèves Françaises ont 10 €'"

> Hilaire
>
>
> --
> Dr. Geo
> http://drgeo.eu
>
> <test.fgeo><PharoDebug.log>


Reply | Threaded
Open this post in threaded view
|

Re: P7 regression

HilaireFernandes
[...]
stream := WriteStream on: (String new: 4000).
DrGeoXml new
   app: self app;
   saveOn: stream.
[...]

DrGeoXml>>saveOn: stream
    | doc writer root |
    doc := XMLDocument new version: '1.0'.
    writer := XMLWriter on: stream.
    root := XMLElement named: #drgenius.
    self writeFigureAsXmlTo: root.
    doc addElement: root.
    doc printXMLOn: writer.

The stream is passed to the XMLWriter, then it goes down to each items
of the sketch to write its own XML node.

For example, to write its basic information as its name, where I have issue:

writeAsXmlTo: aNode
    "return the newly created element"
    | node |
    self rehash.
    node :=  XMLElement named: self basicType attributes: Dictionary new.
    node attributeAt: #type put: self nodeType;
        attributeAt: #name put: (name ifNil: ['']);
        attributeAt: #id put: self hash asString.
    self writeParentsAsXmlTo: node.
    aNode addElement: node.
    ^node

This code is as is since about 2005-2007

I will dig in it but it is strange.


Le 24/07/2019 à 18:45, Sven Van Caekenberghe a écrit :
> But how do you write it to a file then ?
>
> FileLocator desktop / 'hilaire.xml' writeStreamDo: [ :out | out << 'Les élèves Françaises ont 10 €' ].
>
> (FileLocator desktop / 'hilaire.xml') contents.
>
> ">> 'Les élèves Françaises ont 10 €'"

--
Dr. Geo
http://drgeo.eu



Reply | Threaded
Open this post in threaded view
|

Re: P7 regression

Pharo Smalltalk Users mailing list
Hi Hilaire

I downloaded latest Dr Geo and took a quick look at how the files are saved.
It looks like the xml contents are not encoded (in UTF-8) but simply written to a binary write stream, which means that when a WideString is written you should consistently get a badly encoded file.

You could test this out by changing this code
DrGDirectoryLocal>>put: stream into: filename
        (location  asFileReference / filename) ensureDelete binaryWriteStreamDo: [ :fileStream |
                fileStream nextPutAll: stream contents]


to rather be:
DrGDirectoryLocal>>put: stream into: filename
        (location asFileReference / filename) ensureDelete
                writeStreamEncoded: 'utf-8'
                do: [ :fileStream | fileStream nextPutAll: stream contents ]

When I made this modification it worked for me i.e. the file was readable and correctly encoded.

Regards
Carlo

On 24 Jul 2019, at 21:07, Hilaire <[hidden email]> wrote:

[...]
stream := WriteStream on: (String new: 4000).
DrGeoXml new
   app: self app;
   saveOn: stream.
[...]

DrGeoXml>>saveOn: stream
    | doc writer root |
    doc := XMLDocument new version: '1.0'.
    writer := XMLWriter on: stream.
    root := XMLElement named: #drgenius.
    self writeFigureAsXmlTo: root.
    doc addElement: root.
    doc printXMLOn: writer.

The stream is passed to the XMLWriter, then it goes down to each items
of the sketch to write its own XML node.

For example, to write its basic information as its name, where I have issue:

writeAsXmlTo: aNode
    "return the newly created element"
    | node |
    self rehash.
    node :=  XMLElement named: self basicType attributes: Dictionary new.
    node attributeAt: #type put: self nodeType;
        attributeAt: #name put: (name ifNil: ['']);
        attributeAt: #id put: self hash asString.
    self writeParentsAsXmlTo: node.
    aNode addElement: node.
    ^node

This code is as is since about 2005-2007

I will dig in it but it is strange.


Le 24/07/2019 à 18:45, Sven Van Caekenberghe a écrit :
> But how do you write it to a file then ?
>
> FileLocator desktop / 'hilaire.xml' writeStreamDo: [ :out | out << 'Les élèves Françaises ont 10 €' ].
>
> (FileLocator desktop / 'hilaire.xml') contents.
>
> ">> 'Les élèves Françaises ont 10 €'"

--
Dr. Geo
http://drgeo.eu





Reply | Threaded
Open this post in threaded view
|

Re: P7 regression

Sven Van Caekenberghe-2
That is correct, sk.

#writeStreamDo: is enough though, #utf8 is the default encoding

> On 24 Jul 2019, at 22:16, sk via Pharo-users <[hidden email]> wrote:
>
>
> From: sk <[hidden email]>
> Subject: Re: [Pharo-users] P7 regression
> Date: 24 July 2019 at 22:16:50 GMT+2
> To: Any question about pharo is welcome <[hidden email]>
>
>
> Hi Hilaire
>
> I downloaded latest Dr Geo and took a quick look at how the files are saved.
> It looks like the xml contents are not encoded (in UTF-8) but simply written to a binary write stream, which means that when a WideString is written you should consistently get a badly encoded file.
>
> You could test this out by changing this code
> DrGDirectoryLocal>>put: stream into: filename
> (location  asFileReference / filename) ensureDelete binaryWriteStreamDo: [ :fileStream |
> fileStream nextPutAll: stream contents]
>
>
> to rather be:
> DrGDirectoryLocal>>put: stream into: filename
> (location asFileReference / filename) ensureDelete
> writeStreamEncoded: 'utf-8'
> do: [ :fileStream | fileStream nextPutAll: stream contents ]
>
> When I made this modification it worked for me i.e. the file was readable and correctly encoded.
>
> Regards
> Carlo
>
> On 24 Jul 2019, at 21:07, Hilaire <[hidden email]> wrote:
>
> [...]
> stream := WriteStream on: (String new: 4000).
> DrGeoXml new
>   app: self app;
>   saveOn: stream.
> [...]
>
> DrGeoXml>>saveOn: stream
>    | doc writer root |
>    doc := XMLDocument new version: '1.0'.
>    writer := XMLWriter on: stream.
>    root := XMLElement named: #drgenius.
>    self writeFigureAsXmlTo: root.
>    doc addElement: root.
>    doc printXMLOn: writer.
>
> The stream is passed to the XMLWriter, then it goes down to each items
> of the sketch to write its own XML node.
>
> For example, to write its basic information as its name, where I have issue:
>
> writeAsXmlTo: aNode
>    "return the newly created element"
>    | node |
>    self rehash.
>    node :=  XMLElement named: self basicType attributes: Dictionary new.
>    node attributeAt: #type put: self nodeType;
>        attributeAt: #name put: (name ifNil: ['']);
>        attributeAt: #id put: self hash asString.
>    self writeParentsAsXmlTo: node.
>    aNode addElement: node.
>    ^node
>
> This code is as is since about 2005-2007
>
> I will dig in it but it is strange.
>
>
> Le 24/07/2019 à 18:45, Sven Van Caekenberghe a écrit :
>> But how do you write it to a file then ?
>>
>> FileLocator desktop / 'hilaire.xml' writeStreamDo: [ :out | out << 'Les élèves Françaises ont 10 €' ].
>>
>> (FileLocator desktop / 'hilaire.xml') contents.
>>
>> ">> 'Les élèves Françaises ont 10 €'"
>
> --
> Dr. Geo
> http://drgeo.eu
>
>
>
>
>
>
>


Reply | Threaded
Open this post in threaded view
|

Re: P7 regression

HilaireFernandes
In reply to this post by Pharo Smalltalk Users mailing list
Thanks for the tips. It is definitely a left over from porting DrGeo to
P7 from P4/5/6 (not sure)

But it can't really work as put:into: is used to write both sktech
content (XML file) and its PNG preview.

At some point in Pharo history both UTF8 and bitmap were using binary
stream.

Likely a putBinary:into: is needed too.

--
Dr. Geo
http://drgeo.eu



Reply | Threaded
Open this post in threaded view
|

Re: P7 regression

Sven Van Caekenberghe-2
If you have a character read or write stream, you can use #wrappedStream to access the underlying binary stream, should you need to do that.

> On 25 Jul 2019, at 20:31, Hilaire <[hidden email]> wrote:
>
> Thanks for the tips. It is definitely a left over from porting DrGeo to
> P7 from P4/5/6 (not sure)
>
> But it can't really work as put:into: is used to write both sktech
> content (XML file) and its PNG preview.
>
> At some point in Pharo history both UTF8 and bitmap were using binary
> stream.
>
> Likely a putBinary:into: is needed too.
>
> --
> Dr. Geo
> http://drgeo.eu
>
>
>


Reply | Threaded
Open this post in threaded view
|

Re: P7 regression

HilaireFernandes
I don't understand how #wrappedStream works in my situation?

Is #isBinary not good enougth?

Le 25/07/2019 à 21:53, Sven Van Caekenberghe a écrit :

> If you have a character read or write stream, you can use #wrappedStream to access the underlying binary stream, should you need to do that.
>
>> On 25 Jul 2019, at 20:31, Hilaire <[hidden email]> wrote:
>>
>> Thanks for the tips. It is definitely a left over from porting DrGeo to
>> P7 from P4/5/6 (not sure)
>>
>> But it can't really work as put:into: is used to write both sktech
>> content (XML file) and its PNG preview.
>>
>> At some point in Pharo history both UTF8 and bitmap were using binary
>> stream.
>>
>> Likely a putBinary:into: is needed too.
>>
>> --
>> Dr. Geo
>> http://drgeo.eu
>>
>>
>>
>
--
Dr. Geo
http://drgeo.eu



Reply | Threaded
Open this post in threaded view
|

Re: P7 regression

Sven Van Caekenberghe-2
You said you wanted to write both character data (XML) as binary data (images) to the same stream, or so I understood.

> On 26 Jul 2019, at 00:22, Hilaire <[hidden email]> wrote:
>
> I don't understand how #wrappedStream works in my situation?
>
> Is #isBinary not good enougth?
>
> Le 25/07/2019 à 21:53, Sven Van Caekenberghe a écrit :
>> If you have a character read or write stream, you can use #wrappedStream to access the underlying binary stream, should you need to do that.
>>
>>> On 25 Jul 2019, at 20:31, Hilaire <[hidden email]> wrote:
>>>
>>> Thanks for the tips. It is definitely a left over from porting DrGeo to
>>> P7 from P4/5/6 (not sure)
>>>
>>> But it can't really work as put:into: is used to write both sktech
>>> content (XML file) and its PNG preview.
>>>
>>> At some point in Pharo history both UTF8 and bitmap were using binary
>>> stream.
>>>
>>> Likely a putBinary:into: is needed too.
>>>
>>> --
>>> Dr. Geo
>>> http://drgeo.eu
>>>
>>>
>>>
>>
> --
> Dr. Geo
> http://drgeo.eu
>
>
>


Reply | Threaded
Open this post in threaded view
|

Re: P7 regression

HilaireFernandes
Not at the same time on the same file. #put:into is use in one situation
to write out a sketch as a XML file, and in another situation to write
out a PNG image.

However, It is true DrGeo xml document can embed bitmap, but it is
base64 encoded, so saving with readStream is not a problem.

I am still curious how I will use wrappedStream in  case DrGeo's XML
document contained binary data.


Le 26/07/2019 à 08:03, Sven Van Caekenberghe a écrit :

> You said you wanted to write both character data (XML) as binary data (images) to the same stream, or so I understood.
>
>> On 26 Jul 2019, at 00:22, Hilaire <[hidden email]> wrote:
>>
>> I don't understand how #wrappedStream works in my situation?
>>
>> Is #isBinary not good enougth?
>>
>> Le 25/07/2019 à 21:53, Sven Van Caekenberghe a écrit :
>>> If you have a character read or write stream, you can use #wrappedStream to access the underlying binary stream, should you need to do that.
>>>
>>>> On 25 Jul 2019, at 20:31, Hilaire <[hidden email]> wrote:
>>>>
>>>> Thanks for the tips. It is definitely a left over from porting DrGeo to
>>>> P7 from P4/5/6 (not sure)
>>>>
>>>> But it can't really work as put:into: is used to write both sktech
>>>> content (XML file) and its PNG preview.
>>>>
>>>> At some point in Pharo history both UTF8 and bitmap were using binary
>>>> stream.
>>>>
>>>> Likely a putBinary:into: is needed too.
>>>>
>>>> --
>>>> Dr. Geo
>>>> http://drgeo.eu
>>>>
>>>>
>>>>
>> --
>> Dr. Geo
>> http://drgeo.eu
>>
>>
>>
>
--
Dr. Geo
http://drgeo.eu