search

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

transient symbol list maintains reference to newly added symbol dictionary after abort #349

Open ericwinger opened 6 years ago

ericwinger commented 6 years ago

Follow up from #348. Slightly different reproduction case. Done in clean repository. No commits done in image at all. If you clone a repository, then load it, then abort, the project stays in the loadedProjects list.

Reproduction case not using Jadeite (showing all return values)

Rowan image loadedProjects. anIdentitySet( aRwGsLoadedSymbolDictProject Rowan, aRwGsLoadedSymbolDictProject Cypress, aRwGsLoadedSymbolDictProject Tonel, aRwGsLoadedSymbolDictProject STON)

Rowan projectTools clone cloneSpecUrl: 'file:$ROWAN_PROJECTS_HOME/Rowan/samples/RowanSample1.ston' gitRootPath: '$ROWAN_PROJECTS_HOME' useSsh: true. 'A clone for ''RowanSample1'' already exists in ''$ROWAN_PROJECTS_HOME/RowanSample1'', so the clone operation is being skipped The project project has been registered with Rowan at the existing location .'

Rowan image loadedProjects. anIdentitySet( aRwGsLoadedSymbolDictProject RowanSample1, aRwGsLoadedSymbolDictProject Rowan, aRwGsLoadedSymbolDictProject Cypress, aRwGsLoadedSymbolDictProject Tonel, aRwGsLoadedSymbolDictProject STON)

System abortTransaction System

Rowan image loadedProjects. anIdentitySet( aRwGsLoadedSymbolDictProject Rowan, aRwGsLoadedSymbolDictProject Cypress, aRwGsLoadedSymbolDictProject Tonel, aRwGsLoadedSymbolDictProject STON)

!!!!! Correct to here. RowanSample1 correctly removed from loadedProjects list !!!!!!

Rowan projectTools clone cloneSpecUrl: 'file:$ROWAN_PROJECTS_HOME/Rowan/samples/RowanSample1.ston' gitRootPath: '$ROWAN_PROJECTS_HOME' useSsh: true. 'A clone for ''RowanSample1'' already exists in ''$ROWAN_PROJECTS_HOME/RowanSample1'', so the clone operation is being skipped The project project has been registered with Rowan at the existing location .'

Rowan image loadedProjects. anIdentitySet( aRwGsLoadedSymbolDictProject RowanSample1, aRwGsLoadedSymbolDictProject Rowan, aRwGsLoadedSymbolDictProject Cypress, aRwGsLoadedSymbolDictProject Tonel, aRwGsLoadedSymbolDictProject STON)

Rowan projectTools load loadProjectNamed: 'RowanSample1' aRwProjectSetModification

Rowan image loadedProjects. anIdentitySet( aRwGsLoadedSymbolDictProject RowanSample1, aRwGsLoadedSymbolDictProject Rowan, aRwGsLoadedSymbolDictProject Cypress, aRwGsLoadedSymbolDictProject Tonel, aRwGsLoadedSymbolDictProject STON)

System abortTransaction System

Rowan image loadedProjects anIdentitySet( aRwGsLoadedSymbolDictProject RowanSample1, aRwGsLoadedSymbolDictProject Rowan, aRwGsLoadedSymbolDictProject Cypress, aRwGsLoadedSymbolDictProject Tonel, aRwGsLoadedSymbolDictProject STON)

dalehenrich commented 6 years ago

Okay ... this time I can reproduce the bug (the result is 5, when it should be 4):

print
    Rowan image loadedProjects.
    Rowan projectTools clone 
        cloneSpecUrl: 'file:$ROWAN_PROJECTS_HOME/Rowan/samples/RowanSample1.ston' 
        gitRootPath: '$ROWAN_PROJECTS_HOME' 
        useSsh: true.
    Rowan image loadedProjects.
    System abort.
    Rowan image loadedProjects.
    Rowan projectTools clone 
        cloneSpecUrl: 'file:$ROWAN_PROJECTS_HOME/Rowan/samples/RowanSample1.ston' 
        gitRootPath: '$ROWAN_PROJECTS_HOME' 
        useSsh: true.
    Rowan image loadedProjects.
    Rowan projectTools load loadProjectNamed: 'RowanSample1'.
    System abort.
    Rowan image loadedProjects size.
%
[42 sz:0 cls: 74241 SmallInteger] 5 == 0x5

Interestingly enough, if you logout, login and look at loaded projects size again, you get 4 as it should be:

logout
login
print
    Rowan image loadedProjects size.
%
[34 sz:0 cls: 74241 SmallInteger] 4 == 0x4

So something is being kept around in session state and I'll see if I can track it down ...

dalehenrich commented 6 years ago

Okay - this is a good catch ...

TL;DR, logout and login to clean up the bad state.

This has to do with how the temporary and persistent symbol dictionaries are being managed in GemStone and the following script:

    logout
    login
    level 2
    run
    | symbolLists |
    symbolLists := {}.
    Rowan projectTools clone 
        cloneSpecUrl: 'file:$ROWAN_PROJECTS_HOME/Rowan/samples/RowanSample1.ston' 
        gitRootPath: '$ROWAN_PROJECTS_HOME' 
        useSsh: true.
    Rowan projectTools load loadProjectNamed: 'RowanSample1'.
    symbolLists add: 'PRE-ABORT'.
    symbolLists addAll: { Rowan image symbolList collect: [:each | each name ].  System myUserProfile symbolList collect: [:each | each name ] }.
    System abort.
    symbolLists add: 'POST-ABORT'.
    symbolLists addAll: { Rowan image symbolList collect: [:each | each name ].  System myUserProfile symbolList collect: [:each | each name ] }.
    symbolLists
%
[53135873 sz:6 cls: 66817 Array] a Array
  #1 [53136385 sz:9 cls: 74753 String] PRE-ABORT
  #2 [53136897 sz:7 cls: 66817 Array] a Array
    #1 [3962881 sz:11 cls: 110849 Symbol] UserGlobals
    #2 [484097 sz:7 cls: 110849 Symbol] Globals
    #3 [4052481 sz:9 cls: 110849 Symbol] Published
    #4 [13337089 sz:11 cls: 110849 Symbol] RowanKernel
    #5 [13333761 sz:11 cls: 110849 Symbol] RowanLoader
    #6 [13330945 sz:10 cls: 110849 Symbol] RowanTools
    #7 [38779905 sz:16 cls: 110849 Symbol] SampleSymbolDict
  #3 [53139457 sz:7 cls: 66817 Array] a Array
    #1 [3962881 sz:11 cls: 110849 Symbol] UserGlobals
    #2 [484097 sz:7 cls: 110849 Symbol] Globals
    #3 [4052481 sz:9 cls: 110849 Symbol] Published
    #4 [13337089 sz:11 cls: 110849 Symbol] RowanKernel
    #5 [13333761 sz:11 cls: 110849 Symbol] RowanLoader
    #6 [13330945 sz:10 cls: 110849 Symbol] RowanTools
    #7 [38779905 sz:16 cls: 110849 Symbol] SampleSymbolDict
  #4 [53122817 sz:10 cls: 74753 String] POST-ABORT
  #5 [53122305 sz:7 cls: 66817 Array] a Array
    #1 [3962881 sz:11 cls: 110849 Symbol] UserGlobals
    #2 [484097 sz:7 cls: 110849 Symbol] Globals
    #3 [4052481 sz:9 cls: 110849 Symbol] Published
    #4 [13337089 sz:11 cls: 110849 Symbol] RowanKernel
    #5 [13333761 sz:11 cls: 110849 Symbol] RowanLoader
    #6 [13330945 sz:10 cls: 110849 Symbol] RowanTools
    #7 [38779905 sz:16 cls: 110849 Symbol] SampleSymbolDict
  #6 [53120001 sz:6 cls: 66817 Array] a Array
    #1 [3962881 sz:11 cls: 110849 Symbol] UserGlobals
    #2 [484097 sz:7 cls: 110849 Symbol] Globals
    #3 [4052481 sz:9 cls: 110849 Symbol] Published
    #4 [13337089 sz:11 cls: 110849 Symbol] RowanKernel
    #5 [13333761 sz:11 cls: 110849 Symbol] RowanLoader
    #6 [13330945 sz:10 cls: 110849 Symbol] RowanTools

Note that PRE-ABORT the transient and persistent symbol lists are the same size and the the last entry in each case is SampleSymbolDict ... POST-ABORT the transient symbol list still has SampleSymbolDict while the persistent symbol list does not ...

Completely agree that this is confusing.

My desire is to honor symbol dictionaries that only show up the transient symbol list, so at one level this is by design.

Contributing to this is the fact that Rowan is automatically adding a symbol dictionary to the current user's symbol list and I would argue that this is the true root cause of the bug .. as a user if I were to explicitly add a symbol dictionary to my symbol list I would more than likely do a commit, so that I would not want to inadvertently lose my symbol dictionary if I decide to abort, as it could easily lead to the same weird system state that we are seeing here ...

I would further argue that the Rowan sample projects should be using UserGlobals instead of a custom symbol dictionary ...

Finally, I need to revisit the wisdom of automatically adding a symbol dictionary at all ... if users are forced to add the symbol dictionaries that are needed to load a project, then we wouldn't be having this conversation in the first place;)

dalehenrich commented 6 years ago

@ericwinger with this commit b60895e RowanSample1 project is loaded into UserGlobals instead of the new symbol dictionary as a workaround for Issue #349

ericwinger commented 6 years ago

@dalehenrich Cool, I'll make a note to test it after I've integrated v1.2 next.

dalehenrich commented 6 years ago

GemStone internal bug 47755 was reported about this issue and at this point in time, the GemStone bug will not be fixed, so I will want to revisit the wisdom of automatically adding missing symbol dictionaries in Rowan ... which is cool ... there are several different approaches that could be taken ...

ericwinger commented 6 years ago

The simplistic test case now works. I can clone & load RowanSample1, then abort. The project goes away as expected.