> On 17.06.2020, at 01:16, Tobias Pape <Das.Linux at gmx.de> wrote:
> 
>> 
>> On 17.06.2020, at 01:04, tim Rowledge <tim at rowledge.org> wrote:
>> 
>> Here's a fun problem. Part of the 'finger pointing' even points at me for this one...
>> 
>> After loading Seaside into that clean 5.3 image it seems impossible to get Seaside to work. All that happens is a very unhelpful page saying 
>> Internal Error: SystemVersion>>>  major
>> 
>> No amount of trying adaptor restarts etc does anything different. Eventually one realises that the error handling has broken somewhere, in such a way as to prevent it even printing the error properly. This is always a fun thing to solve.
>> 
>> Evidently somewhere a SystemVersion is being sent #major - but it does not implement it. OK, create an implementation and break on it. Dig deep into the stack and... #asMutator is sent to a Symbol, which is deprecated and that triggers the Seaside debug stack printing.
>> 
>> So, problem 1 - the deprecation checking clearly got turned off in my Pi image and I didn't notice adequately to understand what it might change. Damn. Disabling the deprecation warnings lets Seaside 'work' - except of course is still has the problem in the error handling that will bite later.
>> 
>> Where it goes wrong is in WAPharoWalkback>>#tempNamesAndValuesIn:do: that uses
>> ` 	SystemVersion current major >= 8 ifTrue:[`
>> ... which cannot work. Squeak hasn't implemented SystemVersion>>>  major since at least before Squeak 4.0.
>> 
>> I also suspect that despite the WAPharoWalkback class comment claiming it is a Squeak specific class, that in fact this is a Pharo specific class. Something about the class name... I dunno? Also of course, the Squeak major version name isn't 8, it's currently 5.
>> 
>> The good news is that simply changing the method to 
>> tempNamesAndValuesIn: aContext do: aTwoArgumentBlock
>> 	SystemVersion current majorVersionNumber >= 5 ifTrue:[
>> 		aContext tempNames doWithIndex: [ :each :index |
>> 			aTwoArgumentBlock
>> 				value: each
>> 				value: (aContext namedTempAt: index) ] ]
>> lets the Seaside walkback work properly. 
>> 
>> However, it seems odd that there is any guarding of the doWithIndex: clause - was there ever a time that it shouldn't have been evaluated? I don't *think* it would have been a problem pre-Closures but maybe?
> 
> Here are the two relevant commits:
> 
> https://github.com/SeasideSt/Seaside/commit/20e082861098f511fc2fea1bdb463de8560ec25f
> https://github.com/SeasideSt/Seaside/commit/3e430a7d294fdf3784abd62ec55fdfbedd20ffc8#diff-a0aa33163150c439089b33f44dec6a3f
> 
> It seems pharo before 8 used a different API to access the context…
> 
> 
>> Also, what is the best practical option for fixing this permanently? It clearly can't work in a Pharo image so some separation will be required. Maybe add a really-Squeak specific subclass of WAPharoWalkback to the Seaside-Pharo-Development-Core package? Add a(nother) squeak related package to the configuration?
> 
> We have something like these already. Seaside-Squeak-Development-Core is maybe in order.
> Or actually put that in Grease, where GemStone is already doing the right thing, more or less
> 	https://github.com/SeasideSt/Grease/blob/master/repository/Grease-GemStone-Core.package/GsContext.class/instance/tempNames.st
> 

By that, I don't mean the implementation per se but the fact that there's a process abstraction with a "compatible"? api.
It mimics Squeaks process, btw.

I now wonder how the walkbacks have ever worked in Squeak after 2010…

Best regards
	-Tobias


>> 
>> tim