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

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
56 messages Options
123
Reply | Threaded
Open this post in threaded view
|

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

GLASS mailing list
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!

_______________________________________________
Glass mailing list
[hidden email]
http://lists.gemtalksystems.com/mailman/listinfo/glass
Reply | Threaded
Open this post in threaded view
|

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

GLASS mailing list
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!


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


_______________________________________________
Glass mailing list
[hidden email]
http://lists.gemtalksystems.com/mailman/listinfo/glass
Reply | Threaded
Open this post in threaded view
|

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

GLASS mailing list


On Wed, Aug 26, 2015 at 7:40 PM, Dale Henrichs via Glass <[hidden email]> wrote:
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"?


Separate steps (one attempt). I tried others, but failed too. 
 
When you say it didn't work, are you getting errors; or are you ending up with missing classes or incorrect structure?


I mess up the class hierarchy. For instance, those 2 subclases would have as the superclass the superclass of the one expected. Or I would end having 2 kind of the same class but different. Or I would end with a class hierarchy that look correct, but then doing a #listInstanecs: would fail with some problem too. 
 
Hopefully someone who has done something similar to this will step in before I start reading documentation and code:)


Hopefully! I have done class rename migrations in the past. But this one has 2 things I didn't have before:

1) the class I am renaming has subclasses (and subinstances)
2) the class I am renaming is to rename it to a name that already existed in the system (so I am not sure if everything is correctly removed when I delete this class with MC). 

Maybe I should just try to rename it to a different name and see if the problem is either 1) or 2).... But even this (if I would rename it to a different none ever existing name) I still don't know how to do the migration. Would that be automatic (meaning I simply do the rename and do a #bulkMigrate) ? do I need to keep both classes and perform the #addNewVersion: and #migrateInstances: ? if true, which one belongs as the superclass of the subclasses? etc.. 

Thanks Dale, maybe someone jumps in :)


 
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!


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


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




--

_______________________________________________
Glass mailing list
[hidden email]
http://lists.gemtalksystems.com/mailman/listinfo/glass
Reply | Threaded
Open this post in threaded view
|

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

GLASS mailing list
In reply to this post by GLASS mailing list
Mariano,

As I understand it, the class you want to remove has no instances and then you want to simply rename another class. I don’t see that migration is required (though we can revisit that). Consider the following:

Object
- FaSecurityClosingPriceRecord (no instances)
- SpecialSuperclass
- - FaSecurityClosingPriceRecord2
- - - FSCPR2a (instances)
- - - FSCPR2b (instances)

| myClass |
UserGlobals removeKey: #'FaSecurityClosingPriceRecord'.
myClass := UserGlobals removeKey: #'FaSecurityClosingPriceRecord2'.
myClass _beVariantWhile: [myClass name: #'FaSecurityClosingPriceRecord'].
UserGlobals at: #'FaSecurityClosingPriceRecord' put: myClass.

This works with a basic GemStone image; I’m not sure what it does to the various Monticello structures, but you can investigate it. Does that do what you want?

James

On Aug 26, 2015, at 3:16 PM, Mariano Martinez Peck via Glass <[hidden email]> 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!
_______________________________________________
Glass mailing list
[hidden email]
http://lists.gemtalksystems.com/mailman/listinfo/glass


_______________________________________________
Glass mailing list
[hidden email]
http://lists.gemtalksystems.com/mailman/listinfo/glass
Reply | Threaded
Open this post in threaded view
|

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

GLASS mailing list
If this technique does the job, then the monticello structures should be happy as well...

Dale

On 8/26/15 3:59 PM, James Foster via Glass wrote:
Mariano,

As I understand it, the class you want to remove has no instances and then you want to simply rename another class. I don’t see that migration is required (though we can revisit that). Consider the following:

Object
- FaSecurityClosingPriceRecord (no instances)
- SpecialSuperclass
- - FaSecurityClosingPriceRecord2
- - - FSCPR2a (instances)
- - - FSCPR2b (instances)

| myClass |
UserGlobals removeKey: #'FaSecurityClosingPriceRecord'.
myClass := UserGlobals removeKey: #'FaSecurityClosingPriceRecord2'.
myClass _beVariantWhile: [myClass name: #'FaSecurityClosingPriceRecord'].
UserGlobals at: #'FaSecurityClosingPriceRecord' put: myClass.

This works with a basic GemStone image; I’m not sure what it does to the various Monticello structures, but you can investigate it. Does that do what you want?

James

On Aug 26, 2015, at 3:16 PM, Mariano Martinez Peck via Glass <[hidden email]> 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!
_______________________________________________
Glass mailing list
[hidden email]
http://lists.gemtalksystems.com/mailman/listinfo/glass



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


_______________________________________________
Glass mailing list
[hidden email]
http://lists.gemtalksystems.com/mailman/listinfo/glass
Reply | Threaded
Open this post in threaded view
|

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

GLASS mailing list
In reply to this post by GLASS mailing list
On 08/26/2015 03:59 PM, James Foster via Glass wrote:

> Mariano,
>
> As I understand it, the class you want to remove has no instances and
> then you want to simply rename another class. I don’t see that migration
> is required (though we can revisit that). Consider the following:
>
> Object
> - FaSecurityClosingPriceRecord (no instances)
> - SpecialSuperclass
> - - FaSecurityClosingPriceRecord2
> - - - FSCPR2a (instances)
> - - - FSCPR2b (instances)
>
> | myClass |
> UserGlobals removeKey: #'FaSecurityClosingPriceRecord'.
> myClass := UserGlobals removeKey: #'FaSecurityClosingPriceRecord2'.
> myClass _beVariantWhile: [myClass name: #'FaSecurityClosingPriceRecord'].
> UserGlobals at: #'FaSecurityClosingPriceRecord' put: myClass.
>

I recommend against the use of _beVariantWhile: if there's an
alternative -- it's a dangerous tool. In this case there is an
alternative: #changeNameTo:. So you could do this as:

UserGlobals
  removeKey: #'FaSecurityClosingPriceRecord';
  at: #'FaSecurityClosingPriceRecord'
    put: FaSecurityClosingPriceRecord2.
FaSecurityClosingPriceRecord2
  changeNameTo: #'FaSecurityClosingPriceRecord'.
UserGlobals
  removeKey: #'FaSecurityClosingPriceRecord2'.

Regards,

-Martin
_______________________________________________
Glass mailing list
[hidden email]
http://lists.gemtalksystems.com/mailman/listinfo/glass
Reply | Threaded
Open this post in threaded view
|

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

GLASS mailing list
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 :)

Thank you guys, 

On Wed, Aug 26, 2015 at 8:16 PM, Martin McClure <[hidden email]> wrote:
On 08/26/2015 03:59 PM, James Foster via Glass wrote:
> Mariano,
>
> As I understand it, the class you want to remove has no instances and
> then you want to simply rename another class. I don’t see that migration
> is required (though we can revisit that). Consider the following:
>
> Object
> - FaSecurityClosingPriceRecord (no instances)
> - SpecialSuperclass
> - - FaSecurityClosingPriceRecord2
> - - - FSCPR2a (instances)
> - - - FSCPR2b (instances)
>
> | myClass |
> UserGlobals removeKey: #'FaSecurityClosingPriceRecord'.
> myClass := UserGlobals removeKey: #'FaSecurityClosingPriceRecord2'.
> myClass _beVariantWhile: [myClass name: #'FaSecurityClosingPriceRecord'].
> UserGlobals at: #'FaSecurityClosingPriceRecord' put: myClass.
>

I recommend against the use of _beVariantWhile: if there's an
alternative -- it's a dangerous tool. In this case there is an
alternative: #changeNameTo:. So you could do this as:

UserGlobals
  removeKey: #'FaSecurityClosingPriceRecord';
  at: #'FaSecurityClosingPriceRecord'
    put: FaSecurityClosingPriceRecord2.
FaSecurityClosingPriceRecord2
  changeNameTo: #'FaSecurityClosingPriceRecord'.
UserGlobals
  removeKey: #'FaSecurityClosingPriceRecord2'.

Regards,

-Martin



--

_______________________________________________
Glass mailing list
[hidden email]
http://lists.gemtalksystems.com/mailman/listinfo/glass
Reply | Threaded
Open this post in threaded view
|

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

GLASS mailing list
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.

Regards,

-Martin
_______________________________________________
Glass mailing list
[hidden email]
http://lists.gemtalksystems.com/mailman/listinfo/glass
Reply | Threaded
Open this post in threaded view
|

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

GLASS mailing list


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.




--

_______________________________________________
Glass mailing list
[hidden email]
http://lists.gemtalksystems.com/mailman/listinfo/glass
Reply | Threaded
Open this post in threaded view
|

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

GLASS mailing list
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
>
_______________________________________________
Glass mailing list
[hidden email]
http://lists.gemtalksystems.com/mailman/listinfo/glass
Reply | Threaded
Open this post in threaded view
|

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

GLASS mailing list

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
>



--

_______________________________________________
Glass mailing list
[hidden email]
http://lists.gemtalksystems.com/mailman/listinfo/glass
Reply | Threaded
Open this post in threaded view
|

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

GLASS mailing list
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
_______________________________________________
Glass mailing list
[hidden email]
http://lists.gemtalksystems.com/mailman/listinfo/glass
Reply | Threaded
Open this post in threaded view
|

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

GLASS mailing list
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



--

_______________________________________________
Glass mailing list
[hidden email]
http://lists.gemtalksystems.com/mailman/listinfo/glass
Reply | Threaded
Open this post in threaded view
|

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

GLASS mailing list


On Thu, Aug 27, 2015 at 11:37 PM, Mariano Martinez Peck <[hidden email]> wrote:
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. 



OK, I could get the stack now. So...my question is... is this scenario not supposed to work ever with Monticello and I would need to to the change class name trick next time I have same scenario?

Thanks in advance, 


 
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



--



--

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

MCrefactorBug.txt (351K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

GLASS mailing list
In reply to this post by GLASS mailing list


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
Reply | Threaded
Open this post in threaded view
|

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

GLASS mailing list


On Fri, Aug 28, 2015 at 3:22 PM, Dale Henrichs via Glass <[hidden email]> wrote:


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

turning ON or OFF?
 
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:)


Can you confirm that turning OFF automigration solves the example of moving instVars up? 

Thanks!
 
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



--

_______________________________________________
Glass mailing list
[hidden email]
http://lists.gemtalksystems.com/mailman/listinfo/glass
Reply | Threaded
Open this post in threaded view
|

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

GLASS mailing list


On 8/28/15 11:30 AM, Mariano Martinez Peck wrote:
>
>
> Can you confirm that turning OFF automigration solves the example of
> moving instVars up?
>

Sorry if I implied that automigration implied the specific case of
moving instVars up, but I am also under the impression (possibly
mistaken) that the problems that prompted Otto to run that script were
not exclusively due to moving class vars up ...

In other words, I don't believe that turning off automigration solves
that particular problem ... I don't have an automatic answer to that
problem ... because of the limitations of the current Monticello loader
algorithms ...

I do believe (possibly mistaken) that there are additional refactorings
that can cause problems ...

Dale
_______________________________________________
Glass mailing list
[hidden email]
http://lists.gemtalksystems.com/mailman/listinfo/glass
Reply | Threaded
Open this post in threaded view
|

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

GLASS mailing list
To be even more clear ... if "you" expect me to "fix all of the loading
problems" in a vacuum then it will not happen ...

OTOH. If someone takes the time to create a specific set of test cases
that fail in the latest GsDevKit/GLASS then I am willing to work on them
...

The moving variables up issue will probably require a fairly major
rewrite of the Monticello loader (without wading into the problem I
can't guess as to the magnitude of "major rewrite") and my primary
concern is that you guys are hitting other corner cases that I am not
aware of so I want to make sure that I am aware of as many of the
problem areas as possible.

A failing test case is the best way to communicate that to me ...

Dale

On 8/28/15 11:42 AM, Dale Henrichs wrote:

>
>
> On 8/28/15 11:30 AM, Mariano Martinez Peck wrote:
>>
>>
>> Can you confirm that turning OFF automigration solves the example of
>> moving instVars up?
>>
>
> Sorry if I implied that automigration implied the specific case of
> moving instVars up, but I am also under the impression (possibly
> mistaken) that the problems that prompted Otto to run that script were
> not exclusively due to moving class vars up ...
>
> In other words, I don't believe that turning off automigration solves
> that particular problem ... I don't have an automatic answer to that
> problem ... because of the limitations of the current Monticello
> loader algorithms ...
>
> I do believe (possibly mistaken) that there are additional
> refactorings that can cause problems ...
>
> Dale

_______________________________________________
Glass mailing list
[hidden email]
http://lists.gemtalksystems.com/mailman/listinfo/glass
Reply | Threaded
Open this post in threaded view
|

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

GLASS mailing list
In reply to this post by GLASS mailing list


On 8/27/15 8:41 AM, Mariano Martinez Peck via Glass wrote:

>
>
> 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?

Mariano,

The  'they may work on a second pass' error covers a very broad category
of problems if you see this warning and the load did not work properly
(and you want to see the problem fixed:) we need to start characterizing
the specific error conditions and constructing a test case so we can
then move forward and improve the Monticell loader ...

Here is the "horribly complicated" code that is run when loading the
errorDefinitions (`ea` in the code) ... this code addressed the small
set of tests cases that I had at the time but if we are hitting this
area and not making it through without a proper load, then we need to
take a different approach to loading ...:
       [
       [ ea errorLoadOver: (self obsoletionFor: ea) ]
         on: MessageNotUnderstood
         do: [ :ex |
           "avoid problems when loading Issue 246 bugfix"
           ea loadOver: (self obsoletionFor: ea) ] ]
         on: Error
         do: [ :ex |
           "fix for Issue 247"
           ex gsNumber = (ErrorSymbols at: #'rtErrAddDupInstvar')
             ifTrue: [
               | ar superclass |
               ar := nil.
               superclass := Smalltalk
                 at: ea superclassName asSymbol
                 ifAbsent: [ ex pass ].
               [ ar == nil ]
                 whileTrue: [
                   (ar := duplicateInstanceVarDefs at: superclass name
asString ifAbsent: [  ])
                     == nil
                     ifTrue: [
                       (superclass := superclass superclass) == nil
                         ifTrue: [ ex pass ] ] ].
               repairs
                 add: (ar at: 2);
                 addAll: (ar at: 3);
                 yourself.
               repairErrorStream
                 cr;
                 nextPutAll: ex description;
                 cr.
               ex return: nil ].
           ex pass ]

Dale
_______________________________________________
Glass mailing list
[hidden email]
http://lists.gemtalksystems.com/mailman/listinfo/glass
Reply | Threaded
Open this post in threaded view
|

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

GLASS mailing list
In reply to this post by GLASS mailing list


On 8/27/15 9:03 AM, Otto Behrens via Glass wrote:
> 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.
>

Understood and your feedback is appreciated ... when you try again,
let's try to characterize the problems that show up so that we have a
chance of improving things ... I don't think things are perfect, but I
don't know the particulars of the problems that cause you to do the
workaround of loading the packages a second time especially given the
fact that the original load is (presumably) loading without errors ...
I'd like to plug those holes ...

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