Grrrr cannot migrate (class rename with subclasses and with a name of a deleted class)

Mariano,

I'm not a class migration expert ... I'm aware of some of the special problems/limitations of the auto migration code, but I really haven't studied the ins/outs of custom migration plans...

With that said, I will take a crack at helping ...

First a question: are 1), 2), 3) three separate steps along the way to a single end or are they three separate attempts to "solve the problem"?

When you say it didn't work, are you getting errors; or are you ending up with missing classes or incorrect structure?

Hopefully someone who has done something similar to this will step in before I start reading documentation and code:)

Dale

On 8/26/15 3:16 PM, Mariano Martinez Peck via Glass wrote:

Hi guys,

Sorry to bother again but I really can't migrate. I tried everything, nothing worked. I admit I have a weird scenario, but still.

- I have a class FaSecurityClosingPriceRecord which has no instance nor subclasses. 

- I have a separate class called FaSecurityClosingPriceRecord2 (yes, don't ask please) which a special superclass and also have 2 subclasses (this is a different hierarchy than  than FaSecurityClosingPriceRecord). Both subclasses do have instances. 

So...what I need to do is remove old class FaSecurityClosingPriceRecord and rename FaSecurityClosingPriceRecord2 to FaSecurityClosingPriceRecord. And of course, migrating the sub instances.  I tried the following (in order)

1) First monticello commit only the removal of FaSecurityClosingPriceRecord. Then upload code, commit and #doBulkMigrate.

2) Monticello commit where I rename FaSecurityClosingPriceRecord2 to FaSecurityClosingPriceRecord (so subclasses now are subclass of FaSecurityClosingPriceRecord). And I made a copy of FaSecurityClosingPriceRecord and call it FaSecurityClosingPriceRecord2. So to sum up, I have both classes (being equal) FaSecurityClosingPriceRecord and FaSecurityClosingPriceRecord2 and just that FaSecurityClosingPriceRecord is the one that has the subclasses.

3) Then I tried something like:

FaSecurityClosingPriceRecord2 addNewVersion: FaSecurityClosingPriceRecord. System commit. 

FaSecurityClosingPriceRecord2 migrateInstancesTo: FaSecurityClosingPriceRecord.

FaSecurityClosingPriceRecord2 removeFromSystem. 

But that didn't work at all. 

So...how should be the steps I can do to make this refactor/migration? I found no example of classes with subclasses and renaming one in the middle of hierarchy in the guide, so I wonder if this is a special case or not.

Thanks in advance!

--

Mariano
http://marianopeck.wordpress.com

_______________________________________________
Glass mailing list
[hidden email]
http://lists.gemtalksystems.com/mailman/listinfo/glass

_______________________________________________
Glass mailing list
[hidden email]
http://lists.gemtalksystems.com/mailman/listinfo/glass

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.

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.

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

HTH

On Thu, Aug 27, 2015 at 2:57 AM, Mariano Martinez Peck via Glass
<[hidden email]> wrote:
>
>
> On Wed, Aug 26, 2015 at 8:51 PM, Martin McClure
> <[hidden email]> 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
> [hidden email]
> http://lists.gemtalksystems.com/mailman/listinfo/glass
>

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 <[hidden email]> 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
<[hidden email]> wrote:
>
> On Thu, Aug 27, 2015 at 3:44 AM, Otto Behrens <[hidden email]> 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
>> <[hidden email]> wrote:
>> >
>> >
>> > On Wed, Aug 26, 2015 at 8:51 PM, Martin McClure
>> > <[hidden email]> 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
>> > [hidden email]
>> > http://lists.gemtalksystems.com/mailman/listinfo/glass
>> >
>
>
>
>
> --
> Mariano
> http://marianopeck.wordpress.com

--

Mariano
http://marianopeck.wordpress.com

On 8/26/15 11:44 PM, Otto Behrens via Glass wrote:

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

Well obviously, this should not be necessary - and I understand that when you are in the heat of development you have no time to characterize problems:)

I use Metacello all of the time in GemStone (not Pharo) and I don't find it necessary reload all of the packages ... but that does not mean you don't have good reasons for doing this:)

I _will_ ask on of my standard questions - are you using the latest version of Metacello and glassdb from GitHub?

Now that we've got this out of the way, I do know that there are certain sets of refactorings via Monticello loads that can cause problems for GLASS/GsDevKit  (moving instance variables up the hierarchy is the main example) ... turning

autoMigration can reduce some of the problems (GsDeployer class>>bulkMigrate: turns off automigration) - with autoMigration turned off, subclass recompilation is suspended, but I guess there could be traps there as well ... I need to have a better set of test cases and I need to be aware of these problems and I need to find the time to address them:)

I am surprised that you have problems "everytime you load using Metacello" - if you are doing this in self defense then I understand, but if you are truly seeing issues every time, I would like to fix that pretty quickly ...

The standard Monticello package loader used by GemStone is pretty old and pretty simplistic and I suppose we could invent a better loader, but I would need some good examples of the refactorings that cause problems so we can set out with a set of the hard problems in our test suite ...

Dale

_______________________________________________
Glass mailing list
[hidden email]
http://lists.gemtalksystems.com/mailman/listinfo/glass