GemTalk / Rowan

a new project/package manager for Smalltalk that supports FileTree and Tonel repositories, and is independent of Monticello and Metacello
MIT License
14 stars 7 forks source link

(v3) loading projects with method ownership conflicts has issues #917

Closed dalehenrich closed 6 months ago

dalehenrich commented 8 months ago

Working on Rowan4GsDevKit, I hit a number of loader errors related to GsDevKit overrides of base methods.

In Rowan4GsDevKit, I am loading a full GsDevKit image worth of filetree projects normally loaded as DataCurator into a Rowan 3 image as SystemUser (bypassing session methods). There are a large number of base method overrides in the GsDevKit ecosystem (and at least one class override), so collisions were anticipated, however, there a couple of outright errors as well as performance problems that need to be addressed ...

dalehenrich commented 7 months ago

Reopening ... hit snag in doing a full build with these changes, so additional work is needed ... Also need to make sure that before we are done with this issue that we have test cases where method ownership is moved back forth between projects and packages, with special emphasis on moving definitions into an incoming packaging while deleting the original loaded package ...

dalehenrich commented 7 months ago

See bug #752 for related issues ...

dalehenrich commented 7 months ago

with 3ba45c0 ... rowan3 expected tests are passing

dalehenrich commented 7 months ago

Continuing work on Rowan4GsDevKit, I am hitting errors where the class Object or TestCase has been removed from tempSymbols during load process (the classes weren't removed, because the load did not complete), but it is clear that I need ClassDefinition tests as well:

[RwPrjLoadToolV2] Loaded project gemstoneBaseImage manages the class definition of Object, incoming project gemstoneBaseImage and package Filein4Rowan modifies the class.
[RwPrjLoadToolV2] Moving class definition Object from package Filein4Rowan into the package Filein4Rowan
...
[RwPrjLoadToolV2] Loaded project gemstoneBaseImage manages the class definition of TestCase, incoming project gemstoneBaseImage and package Filein2CInit modifies the class.
[RwPrjLoadToolV2] Moving class definition TestCase from package Filein2CInit into the package Filein2CInit

These are not the only ClassDefinitions following this pattern ... it is notable that this pattern appears to be showing up for existing classes that are extended by other packages (with conflicting changes ... presumably) ...

The fact that gemstoneBaseImage project is showing up in the list implies that from the above examples that methods in the Filein4Rowan and Filein2CInit packages contain conflicting definitions ..

New tests are definitely called for, because I know for a fact that the TestCase and Object classes are not changed by incoming packages ...

dalehenrich commented 6 months ago

Issue #920 resolved outstanding issue with writing Filetree format when case insensitive filenames needed

dalehenrich commented 6 months ago

NOTE ... active work is on branch issue_917 ...

Need the following changes in 3.7.2 base ,,, when integrated:

bosch:f_37x>git status
On branch 3.7.2
Your branch is up to date with 'origin/3.7.2'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
    modified:   image/fileinrowan3.topaz

no changes added to commit (use "git add" and/or "git commit -a")
bosch:f_37x>git diff
diff --git a/image/fileinrowan3.topaz b/image/fileinrowan3.topaz
index e812c4363..a2eb7e026 100644
--- a/image/fileinrowan3.topaz
+++ b/image/fileinrowan3.topaz
@@ -354,9 +354,18 @@ run
        loadSpecs := gemstoneLoadedProject loadedLoadSpecifications.
        (loadSpecs specForProjectNamed: gemstoneProjectName)
                addCustomConditionalAttributes: #( 'x509' ).    
-       [ 
-               loadSpecs read load ] 
-                       on: RwExecuteClassInitializeMethodsAfterLoadNotification 
+[
+        [ 
+" Use of RwExistingVisitorChangingPackageOwnershipNotification is required when Rowan:issue_917 is integrated
+               [ loadSpecs read load ]
+                       on: RwExistingVisitorChangingPackageOwnershipNotification
+                        do: [:ex |
+                               ""preserve loaded packaging""
+                               ex resume: false ].
+"
+               loadSpecs read load
+       ] 
+      on: RwExecuteClassInitializeMethodsAfterLoadNotification 
                        do: [:ex |
                                (ex candidateClass isVersionOf: Upgrade3Init)
                                        ifTrue: [ 
@@ -368,6 +377,7 @@ run
                                                "no initializers should run during this step, but for now we'll take 
                                                        our chances"
                                                ex pass ]].
+] on: Error do: [:ex | System waitForDebug ].
   System commitTransaction.    
 % 
 time
dalehenrich commented 6 months ago

Rowan4GsDevKit CI jobs running clean and Rowan 3 expected tests running clean ... merge code int masterV3.2