ObjectAsMethodWrapper >> #uninstall breaks class organisation

ObjectAsMethodWrapper's test suite breaks ObjectAsMethodWrapperDummy's
organization - its #foo and #bar methods disappear even though the
method dictionary still has them. As a result, after you've run OAMW's
tests, you can't do anything with the OAMW PackageInfo - you can't
check to see what changes you made, save the new mcz, etc.

The bug affects both Squeak and Pharo: when you try do anything with
the package you get a MNU because the MethodReferences to
ObjectAsMethodWrapperDummy >> #foo and #bar have nils for their
category instvars.

The attached mcz changes the _ assignments to := ones so Pharo folk
can run the tests.

I don't have a solution yet, but wanted to raise the issue.

It looks like PackageInfo possibly isn't catching the AddEvent sent by
ClassDescription >> #addSelector:withMethod:notifying: but I'm just
guessing: I don't know enough about this part of the system.

frank

On 23 August 2012 10:24, Frank Shearar <[hidden email]> wrote: I've attached a test demonstrating the bug.

> frank

On 23 August 2012 12:49, Bert Freudenberg <[hidden email]> wrote: OK. So the organizer loses the reference because ObjectAsMethodWrapper
calls #removeSelectorSilently:, which eventually calls
ClassDescription >> #removeSelector:. It re-adds the now-unwrapped
CompiledMethod to the method dictionary with #addSelector:withMethod:
which eventually calls ClassDescription >>
#basicAddSelector:withMethod:. However, this does _not_ add the
selector to the organization's elementArray, and Bad Things Happen.

I'm trying to find how things get _into_ a Categorizer's elementArray.
I suspect it's via ClassDescription >>
#addAndClassifySelector:withMethod:inProtocol:notifying:. A Browser
would do this via accepting code, while if you're hacking on the
method dictionary directly, you'll need to do this yourself.

This looks like it would do the trick. At least, it makes the failing test pass:

ObjectAsMethodWrapper >> uninstall
        | category |
        oldMethod isNil ifTrue: [^self].
        category := wrappedClass organization categoryOfElement: selector.
"<-- store the original category"
        wrappedClass removeSelectorSilently: selector.
        wrappedClass addSelector: selector withMethod: oldMethod.
        wrappedClass organization classify: selector under: category. "<--
categorise the restored method."

frank

> This is different in Monticello - if you click "browse" in MC it makes a package snapshot by asking PackageInfo, which iterates through the method dictionary. After running the test this blows up, because it finds the "bar" method, but that is not in the organizer, so its category is nil and you get a walkback.
>
> Events are only used to mark the Monticello working copy dirty. Not to actually keep track of method additions.
>
> - Bert -
>
>
>