[Glass] Grrrr cannot migrate (class rename with subclasses and with a name of a deleted class)
Mariano Martinez Peck via Glass
glass at lists.gemtalksystems.com
Thu Aug 27 19:37:04 PDT 2015
Hi guys,
Sorry to continue with this topic but since I loose quite some time I must
understand what is going on so that it doesn't happen again. For all my
testing, the ONLY way I had make it work is with James/Martin code to
manually hack the symbol list and change class name. I will try to
summarize what is going on.
The repository currently has this:
Object
*- FaSecurityClosingPriceRecord (no instances)*
- SpecialSuperclass
*- - FaSecurityClosingPriceRecord2*
- - - FSCPR2a (instances)
- - - FSCPR2b (instances)
And I am trying to commit:
Object
- SpecialSuperclass
*- - FaSecurityClosingPriceRecord*
- - - FSCPR2a (instances)
- - - FSCPR2b (instances)
If I run James code:
UserGlobals
removeKey: #'FaSecurityClosingPriceRecord';
at: #'FaSecurityClosingPriceRecord'
put: FaSecurityClosingPriceRecord2.
FaSecurityClosingPriceRecord2
changeNameTo: #'FaSecurityClosingPriceRecord'.
UserGlobals
removeKey: #'FaSecurityClosingPriceRecord2'.
And then I load the code via Monticello, I have NO warning from MC and
everything is correct.
If I do NOT run James above code, then I get a warning saying: "there is a
problem with FaSecurityClosingPriceRecord .... it MAY work on a second
pass". So I click proceed, and I load everything it again. No warning this
time. But.... the resulting stuff is broken. I basically end up having 2
classes and 2 metaclasses of FaSecurityClosingPriceRecord and 2 classes.
One associated with the already existing instances and one which is the in
Smalltalk globals and it's the one used for new instances. Confirmation of
this issue:
| metas |
metas := (Metaclass3 allInstances select: [:each | each theNonMetaClass
name = #FaSecurityAdjustedClosingPriceRecord ]).
metas do: [:each | Transcript show: each theNonMetaClass asOop; cr].
metas do: [:each | Transcript show: each theMetaClass asOop; cr].
Prints:
5368758017
21054977
5368758273
13494738945
Now..what is interesting was the Monticello warning I got which I did NOT
get when running james code before. Tracing that is cryptic because
Monticello kind of etas the exception and all it does is the add the
definition to a list of error definitions..... After putting some
flags/halts in #tryToLoad: I found out that indeed the definition in
question was the class definition of FaSecurityAdjustedClosingPriceRecord.
Why it failed?
Well, as part of the class definition it does "MCPlatformSupport
migrateInstancesWithSubclassesOf: cls" and that ends up with a problem in
the
result addAll: (diskBlock value: scanSetThisTime ).
from #_doListInstancesFrom: inputSet with: diskBlock includeMemory:
inMemoryBool
The problem is in fact that "(diskBlock value: scanSetThisTime )" answers
nil.
scanSetThisTime of course is "anIdentitySet(
FaSecurityAdjustedClosingPriceRecord)"
diskBlock closure is this:
'"This is source for a block. "
^ [ :scanSetThisTime |
self _scanPomWithMaxThreads: maxThreads waitForLock: 60 pageBufSize: 8
percentCpuActiveLimit: aPercentage
identSet: scanSetThisTime limit: aSmallInt
scanKind: code toDirectory: directoryString
]'
So...clearly the one that decided to use the word "MAY work" in the error
description was wise!
Damn...I just tried to copy the stack and failed. grrr will see if I can
get it again.
Any ideas?
On Thu, Aug 27, 2015 at 1:03 PM, Otto Behrens <otto at finworks.biz> wrote:
> Can't really say for sure. We found that reloading using Metacello did
> not work. Somehow it picks up that a package did not change and then
> do not reload it. The only way we could force a reload was using
> Gofer.
>
> We did find strange behaviour when using refactoring tools (and we use
> it extensively!) re. this, but could not pinpoint what it was. This
> especially became apparent when several developers edit the same
> package; we found that we lost code / overwrote another developers
> code.
>
> Doing this "reload" is extremely painful as we load code into GS and
> Pharo many times a day. It takes quite some time to do.
>
> For all the guys out there that work on metacello, please forgive us
> for not finding these issues and bypassing them in this way. At some
> point things just got overwhelming. Now that we are running GS 3.2 and
> a fairly recent Seaside / Magritte, we should probably upgrade from
> Pharo 3.0 as well. And then try again.
>
> On Thu, Aug 27, 2015 at 5:41 PM, Mariano Martinez Peck
> <marianopeck at gmail.com> wrote:
> >
> > On Thu, Aug 27, 2015 at 3:44 AM, Otto Behrens <otto at finworks.biz> wrote:
> >>
> >> Hi,
> >>
> >> I would consider doing it in 2 steps. We normally rename one of the
> >> classes first and then deploy. In the next week's deployment cycle, we
> >> do the other half. That way we are sure that all references are fixed
> >> up correctly and that all instances are migrated.
> >>
> >> Did not know about the changeNameTo: trick though. Not sure how I
> >> would use it because Monticello will not necessarily pick up a change
> >> and recompile.
> >>
> >> In our system we really battled with references not being recompiled
> >> and with strange behaviour in Pharo when loading code using Metacello.
> >> And we really should try to find the problem, but we already spent way
> >> too much time on this.
> >>
> >> This may help you when using changeNameTo:, so, this is what we do
> >> every time after loading code with Metacello:
> >>
> >> | gofer |
> >> gofer := Gofer it.
> >> gofer disablePackageCache.
> >> gofer repository: self localWonkaCodeRepository.
> >> allPackageNames doWithIndex: [ :packageName :index |
> >> (MCWorkingCopy registry values anySatisfy: [ :workingCopy |
> >> workingCopy package name asSymbol == packageName asSymbol ])
> >> ifTrue: [ gofer package: packageName ] ].
> >> gofer load
> >>
> >> Where allPackageNames is:
> >> ((MetacelloMCVersionSpecLoader on: <config class> latestVersion spec)
> >> required: {aGroupName};
> >> resolvePackageNames;
> >> packages) values
> >>
> >
> > Hi Otto,
> >
> > One of the problems I usually have when I do refactors in a class
> hierarchy
> > is the 'they may work on a second pass' error. It looks to me that
> > executing the gofer load of all my packages AFTER metacello is an
> automatic
> > workaround for this rather than having to re-run the whole metacello
> load.
> > Do you remember if one of the reason you run that code after metacello is
> > also that error?
> >
> > Best,
> >
> >
> >>
> >> HTH
> >>
> >> On Thu, Aug 27, 2015 at 2:57 AM, Mariano Martinez Peck via Glass
> >> <glass at lists.gemtalksystems.com> wrote:
> >> >
> >> >
> >> > On Wed, Aug 26, 2015 at 8:51 PM, Martin McClure
> >> > <martin.mcclure at gemtalksystems.com> wrote:
> >> >>
> >> >> On 08/26/2015 04:39 PM, Mariano Martinez Peck wrote:
> >> >> > I am very very glad I asked. Thank you very much guys. Cannot
> believe
> >> >> > so
> >> >> > simply it actually was.
> >> >> > So..to my defense, I didn't know a class rename was possible
> without
> >> >> > going with the migration code :)
> >> >>
> >> >> This simplicity is not really very obvious.
> >> >>
> >> >> And it's not *quite* as simple as we indicated. You should also check
> >> >> for code references to the class name, and edit those methods as
> >> >> needed.
> >> >> If a method references "FaSecurityClosingPriceRecord2" it's going to
> >> >> fail the next time you need to recompile it after changing the name
> of
> >> >> the class, and if it references "FaSecurityClosingPriceRecord" and
> was
> >> >> compiled before you made the change it still references the class you
> >> >> deleted.
> >> >>
> >> >
> >> >
> >> > mmmmm I am wondering about the "and if it references
> >> > "FaSecurityClosingPriceRecord" and was compiled before you made the
> >> > change
> >> > it still references the class you deleted."
> >> >
> >> > From Pharo, I did the refactor and so all methods were correctly
> updated
> >> > (those that refer to FaSecurityClosingPriceRecord2 now refer to
> >> > FaSecurityClosingPriceRecord2). So I assume that when loading with MC
> >> > these
> >> > latest code (after having run the above rename code), those methods
> that
> >> > changed will be compiled correctly to the new class. Can you confirm
> >> > this
> >> > please?
> >> >
> >> > Now what happens if I already have methods who refer to
> >> > FaSecurityClosingPriceRecord instead of FaSecurityClosingPriceRecord2.
> >> > Those
> >> > will refer to the old class unless compiled again as Martin says. I do
> >> > have
> >> > none. But still I am curious. What I do not remember is if MC would
> >> > recompile all methods of the package or only those that changed. I
> think
> >> > all
> >> > of them. So...if I re-load the whole app I THINK all methods will be
> >> > recompiled and hence all pointing to the new class (even if they refer
> >> > to
> >> > FaSecurityClosingPriceRecord class before the rename). Do you agree as
> >> > well?
> >> >
> >> > Thanks in advance!!
> >> >
> >> >
> >> >>
> >> >> The reason that a class rename is relatively simple is that a class
> >> >> doesn't use its name for anything important at runtime. The only
> reason
> >> >> that I know of for a class to know its own name is so it can put in a
> >> >> print string for debugging.
> >> >>
> >> >> At compile time, the compiler finds a class by looking up its name in
> >> >> the symbol dictionaries, so in order to compile references to a class
> >> >> you need the dictionary entry. Class names don't really do much more
> >> >> than that -- the instances don't usually care.
> >> >>
> >> >>
> >> >> Of course if you change instance variables, or the superclass
> >> >> hierarchy,
> >> >> then the instances are usually affected and you still must do a
> >> >> migration.
> >> >>
> >> >
> >> >
> >> >
> >> > --
> >> > Mariano
> >> > http://marianopeck.wordpress.com
> >> >
> >> > _______________________________________________
> >> > Glass mailing list
> >> > Glass at lists.gemtalksystems.com
> >> > http://lists.gemtalksystems.com/mailman/listinfo/glass
> >> >
> >
> >
> >
> >
> > --
> > Mariano
> > http://marianopeck.wordpress.com
>
--
Mariano
http://marianopeck.wordpress.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.gemtalksystems.com/mailman/private/glass/attachments/20150827/8566c2d6/attachment-0001.html>
More information about the Glass
mailing list