Hi Nicolas,

> On Apr 8, 2017, at 8:56 AM, Nicolas Cellier <nicolas.cellier.aka.nice at gmail.com> wrote:
> 
> 
> 
> 2017-04-08 17:29 GMT+02:00 Nicolas Cellier <nicolas.cellier.aka.nice at gmail.com>:
>> 
>> 
>> 2017-04-08 17:14 GMT+02:00 Nicolas Cellier <nicolas.cellier.aka.nice at gmail.com>:
>>> 
>>> 
>>> 2017-04-08 14:09 GMT+02:00 David T. Lewis <lewis at mail.msen.com>:
>>>> On Sat, Apr 08, 2017 at 01:59:34PM +0200, Nicolas Cellier wrote:
>>>> > Ah, but now that I've put a
>>>> >     (selector == #unwindTo: and: [ self name = 'Context']) ifTrue:[self
>>>> > halt: 'debug me'].
>>>> > I can't reproduce the problem related to removal of Context methods!!!
>>>> >
>>>> > Instead, I've got the 'Merging Kernel-eem.1078' window with all the
>>>> > MethodContext->Context renaming in conflict
>>>> > I already reported that in another thread where I told that the upgrade
>>>> > went "smoothly" for both my 32 & 64 bits images.
>>>> > I don't know if it's related to package cache, or .mcd, or ???
>>>> >
>>>> > It might also be that package ancestry is not correct du to overwriting of
>>>> > some packages, otherwise we should not get a merge window, but well...
>>>> >
>>>> > Ah, but now I think I understand: it's just because I have pending changes
>>>> > in Kernel due to the addition of halt: above (same for my images which
>>>> > usually have some unpublished experiments).
>>>> > We then trigger a merge rather than a load. And the merge sees conflict
>>>> > probably because of broken ancestry (the GUID are not corresponding), but
>>>> > once resolved manually, it processes smoothly without removing Context
>>>> > methods...
>>>> >
>>>> > I can't instrument code that easily... This will have to wait for another
>>>> > time slot :(
>>>> >
>>>> 
>>>> Nicolas,
>>>> 
>>>> Thanks very much for working on this and for posting your results so far.
>>>> This is not an easy problem to solve.
>>>> 
>>>> Dave
>>>> 
>>>> 
>>> 
>>> Hi Dave,
>>> don't worry, real show stopper obstacles are rare, the rest of them make the game more fun ;)
>>> So I've put the halt in MethodContext class>>removeSelector:  in a *Debug protocol so as to keep Kernel clean, and I've now got the stack at the point of removing what should not be removed.
>>> 
>>> It's in MCDiffyVersion load -> MCVersionLoader load -> MCPackageLoader load.
>>> 
>>> There's a bunch of removals, starting with:
>>> 
>>> an OrderedCollection(a MCMethodDefinition(MethodContext>>unwindTo:)
>>> a MCMethodDefinition(MethodContext>>tryPrimitiveFor:receiver:args:)
>>> a MCMethodDefinition(MethodContext>>tryNamedPrimitiveIn:for:withArgs:)
>>> a MCMethodDefinition(MethodContext>>top)
>>> a MCMethodDefinition(MethodContext>>terminateTo:) ...
>>> 
>>> So yes, this mcd is going to shoot the update process.
>>> Why is it different from the load or merge triggered manually?
>>> Can't we avoid all the method removals and keep only the class removal?
>>> 
>>> I can try this snippet in the MCPackageLoader to reject some superfluous method removals:
>>> 
>>> |  removedClasses |
>>> removedClasses := (removals select: #isClassDefinition) collect: #actualClass.
>>> removedClasses addAll: (removedClasses collect: #class).
>>> removals := removals reject: [:e | e isMethodDefinition and: [removedClasses includes: e actualClass]]
>>> 
>>> Is it OK as a general change in MC?
>>> 
>>> Then I'm not sure how it's going to work, because at this stage, Context is still a subclass of ContextPart as I told before (though it has a different superclass), and #MethodContext is still pointing to Context, so removing either one or the other is going to lead to problems again...
>>> 
>>> So two more hacks might be necessary:
>>> 
>>> [Smalltalk globals at: #MethodContext put: Context copy] on: AttemptToWriteReadOnlyGlobal do: [:exc | exc resume: true].
>>> 
>>> ContextPart instVarAt: 6 put: (ContextPart subclasses select: [:e | e superclass = ContextPart]).
>>> 
>>> Finally, with the 2nd hack, maybe I don't need to forget the superfluous method removals.
>>> However, it failed when I tried, so I've uploaded a Monticello-nice.667 with the hack, and triggered its download in update-eem.406.mcm
>>> Not sure if it is really necessary...
>>> 
>>> Now the update is still failing with an emergency evalutator, and this time it seems to be related to the progress bar with an obsolete block still pointing to MethodContext.
>>> 
>>> ...snip...
>>> UndefinedObject>>handleSignal:
>>> Context(nil)>>handleSignal:
>>> Context(nil)>>handleSignal:
>>> Context(nil)>>handleSignal:
>>> Context(nil)>>handleSignal:
>>> 
>>> I will retry with Bert's hack.
>>> This update hickup is a tough one.
>> 
>> And Bert's hack is not enough. So another possibility would be to move #MethodContext -> Context binding in Undeclared.
>> Undeclared shouldn't auto-clean as long as obsolete block pointing to MethodContext are still on the stack.
>> I'm going to try this.
> 
> Ah yes, this time it worked for me.
> Someone want to try and confirm?

I just updated a 64-bit image from 16987 to 17157 so I think you have it fixed.  Brilliant!

How do we capture your insights in the code?  I'll try and read the thread and the code and see that comments in the code capture things.  But not today; it's raining and we're going to the beach :-). (Muir beach and the Pelican In are fun in the rain too).

_,,,^..^,,,_ (phone)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20170408/e4e9fba1/attachment.html>