Issue 4173 in pharo: If sources file is not present, something bad happens

Status: Accepted
Owner: marianopeck
CC: [hidden email],  [hidden email]
Labels: Milestone-1.3

New issue 4173 by marianopeck: If sources file is not present, something  
bad happens
http://code.google.com/p/pharo/issues/detail?id=4173

Hi guys. In pharo 1.1, 1.2 (in fact, always) one could decide not to use  
the .sources file and instead decompile. At startup the image raises a  
popup saying "ehhhh the .sources couludnt be found" and then after, when  
you browse a method you see the decompiled code.

Now, in Pharo 1.3 I REMOVE all possible .sources and the image doesn't say  
anything at startup. No popup. If I print SourceFiles I get -> an  
ExpandedSourceFileArray(nil  
MultiByteFileStream: '/Users/mariano/Pharo/imagenes/PharoCore-1.2/PharoCore-1.2.changes')    
So...the sources is not there. Two problems:

1) the first one is that the popup doesn't raise.

2) I cannot browse methods because a popup raises saying there was a  
probelm with the sources file: "There may be a problem with your sources  
file!".

I attach screenshot.

So...what could be happening?   I will try to debug it...

I put milestone 1.3 because for production apps it is important to be able  
to run without sources



Attachments:
        Screen shot 2011-05-08 at 9.49.04 PM.png  48.4 KB


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #1 on issue 4173 by marianopeck: If sources file is not present,  
something bad happens
http://code.google.com/p/pharo/issues/detail?id=4173

The same happens with .changes. If I rename it and open the image, I cannot  
browse anymore methods which sourcePointer was pointing to the .changes  
file, and instead I get that ugly popup


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #2 on issue 4173 by marianopeck: If sources file is not present,  
something bad happens
http://code.google.com/p/pharo/issues/detail?id=4173

Ok....finally, after a couple of hours, I found the bug. It is realted to  
the CompiledMethodTrailer and friends.

The problem is in CompiledMethod >> getSourceFor: selector in: class
when the compiled method is a normal one and the .changes / .sources is  
removed, an empty string is returned. Which finally goes to the popup  
saying that there could be a problem with the sources.

So...doing (Mariano >> #self:with:) getSourceFor: #self:with: in: Mariano
will answer ''  if .changes is not present, instead of the decompiled  
method.

This is because CompiledMethod >>#getSourceFor:in:    at the end, manages  
this situtation like this:

               
        ^source
                ifNotNil: [source]
                ifNil: [
                        "Something really wrong -- decompile blind (no temps)"
                         (class decompilerClass new decompile: selector in: class method: self)
                                decompileString]



But in this case, the source is not nil but an empty string. Hence, it was  
not decompield and an empty string was returned. The fix is to change it to:

        source isEmptyOrNil ifTrue: [
                        "Something really wrong -- decompile blind (no temps)"
                        ^ (class decompilerClass new decompile: selector in: class method: self)
                                decompileString].
               
        ^source



Why it used to work before?  (in pharo 1.2 and friends) because there was  
ANOTHER bug that fixes this:

CompiledMethodTrailer >> tempNames
        ^ (kind == #TempsNamesQCompress
                        or: [kind == #TempsNamesZip])
                ifTrue: [data]

So...if it was false, it returned self. But then, in #getSourceFor:in:  you  
have:

        trailer tempNames ifNotNil: [:namesString |
                "Magic sources -- decompile with temp names"
                ^ ((class decompilerClass new withTempNames: namesString)
                                decompile: selector in: class method: self)
                        decompileString].

since "self" was not nil, that piece of code were executed and returned.
Now in Pharo 1.3  tempNames is fixed:

tempNames
        "Answer the string, containing the temps names or nil "
        ^ (kind == #TempsNamesQCompress or: [ kind == #TempsNamesZip ])
                ifTrue: [ data ] ifFalse: [ nil ]

but since nil is returned, it doesn't pass anymore by the decompiler part...

Now....why #getSourceFor:in:  used to compare agains nil instead of empty  
string ? I have no idea.


Please, review this issue.

Name: SLICE-Issue-4173-ProblemWhenSourcesIsNotPresent-MarianoMartinezPeck.1
Author: MarianoMartinezPeck
Time: 14 May 2011, 5:37:31 pm
UUID: 30ce1f86-3832-48bc-a9dd-e94cf90303c2
Ancestors:
Dependencies: Kernel-MarianoMartinezPeck.883

fix to issue 4173



_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Status: FixProposed

Comment #3 on issue 4173 by marianopeck: If sources file is not present,  
something bad happens
http://code.google.com/p/pharo/issues/detail?id=4173

(No comment was entered for this change.)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Status: ReviewNeeded

Comment #4 on issue 4173 by [hidden email]: If sources file is not  
present, something bad happens
http://code.google.com/p/pharo/issues/detail?id=4173

(No comment was entered for this change.)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #5 on issue 4173 by [hidden email]: If sources file is not  
present, something bad happens
http://code.google.com/p/pharo/issues/detail?id=4173

This change using a different subclass of UIManager,
called StartupUIManager, which works mostly like Non-Interactive one,
but defers the #inform: popups to be performed later, when startup is done.

Attachments:
        startup.1.cs  896 bytes


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Status: Closed

Comment #6 on issue 4173 by [hidden email]: If sources file is not  
present, something bad happens
http://code.google.com/p/pharo/issues/detail?id=4173

in 13207


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker