Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DNU when trying to unload an Iceberg pkg where underlying Pharo pkg has been removed #931

Closed
macta opened this issue Jul 29, 2018 · 6 comments
Assignees
Milestone

Comments

@macta
Copy link
Contributor

macta commented Jul 29, 2018

In the Working Copy Of window (which shows you packages you have loaded) if you remove a package in pharo becuase you've refactored - you can still see the package that you loaded.

If you you try to unload it - you get a DNU (its trying to #unload nil) Not sure if the Working Copy window should update when the package is removed, or if we should handle the nil?


UndefinedObject(Object)>>doesNotUnderstand: #unload
IceWorkingCopy>>unloadPackage:
IcePackage>>unload
[ self entity unload ] in [ Iceberg announcer
suspendAllForRepository: self entity repository
while: [ self entity unload ] ] in IceTipPackageModel>>unload in Block: [ self entity unload ]
BlockClosure>>ensure:
IceAnnouncer>>suspendAllMatching:while:
IceAnnouncer>>suspendAllForRepository:while:
[ Iceberg announcer
suspendAllForRepository: self entity repository
while: [ self entity unload ] ] in IceTipPackageModel>>unload in Block: [ Iceberg announcer...
[ :bar |
bar label: aString.
aBlock value ] in MorphicUIManager(UIManager)>>informUser:during: in Block: [ :bar | ...
[ :bar | aBlock value: bar ] in MorphicUIManager>>informUserDuring: in Block: [ :bar | aBlock value: bar ]
BlockClosure>>cull:
[ ^ block cull: self ] in [ self prepareForRunning.
CurrentJob value: self during: [ ^ block cull: self ] ] in Job>>run in Block: [ ^ block cull: self ]
[ activeProcess psValueAt: index put: anObject.
aBlock value ] in CurrentJob(DynamicVariable)>>value:during: in Block: [ activeProcess psValueAt: index put: anObject....
BlockClosure>>ensure:
CurrentJob(DynamicVariable)>>value:during:
CurrentJob class(DynamicVariable class)>>value:during:
[ self prepareForRunning.
CurrentJob value: self during: [ ^ block cull: self ] ] in Job>>run in Block: [ self prepareForRunning....
BlockClosure>>ensure:
Job>>run
MorphicUIManager(UIManager)>>displayProgress:from:to:during:
MorphicUIManager>>informUserDuring:
MorphicUIManager(UIManager)>>informUser:during:
IceTipStandardAction>>basicExecute
[ self basicExecute.
self finishSuccess ] in IceTipStandardAction(IceTipAction)>>execute in Block: [ self basicExecute....
BlockClosure>>on:do:
IceTipStandardAction(IceTipAction)>>withErrorHandlingDo:
IceTipStandardAction(IceTipAction)>>execute
IceTipStandardAction>>execute:
IceTipPackageModel>>unload
IceTipCachedModel>>forwardMessage:

@macta
Copy link
Contributor Author

macta commented Jul 30, 2018

I think something is needed in this area as you do need to be able to unload now missing packages in order to commit a package deletion.

I did a quick fix as follows which sorted things out for my usecase:

IceWorkingCopy>>unloadPackage: aPackage

aPackage mcWorkingCopy ifNotNil: [:p | p unload]

@guillep guillep added this to the 1.2 milestone Jul 31, 2018
@guillep guillep self-assigned this Jul 31, 2018
@guillep guillep closed this as completed Aug 1, 2018
@guillep guillep reopened this Aug 1, 2018
@guillep
Copy link
Member

guillep commented Aug 1, 2018

That's strange. I've checked it in Pharo 7 like this:

  • create a new repository
  • create a package 'NewPackage'
  • Add the package to the repository and commit it
  • remove the package from the system using

'NewPackage' asPackage removeFromSystem

And Iceberg got automatically updated and marked it as "Not loaded".
Then, the UI only offered me the "Remove package" option.

  • What Pharo version are you on? Pharo 6?
  • Did you do something else?

I'll close it by now, feel free to reopen it if you have more info.

@macta
Copy link
Contributor Author

macta commented Aug 1, 2018

This was on Pharo 6.1 with the latest Iceberg installed (from the script). I had a package Exercism-HelloWorld that was appearing top level instead of a sub package of Exercism. Renaming it, made it into a proper sub package, and then renaming it again got back the original name. But this left me with the phantom one I couldn’t delete.

Not sure why you sometimes get those top-level glitches (I’ve seen it a few times).

I will see if I can recreate in 7, or
Maybe this rings some bells?

@guillep
Copy link
Member

guillep commented Aug 1, 2018

This was on Pharo 6.1 with the latest Iceberg installed (from the script). I had a package Exercism-HelloWorld that was appearing top level instead of a sub package of Exercism. Renaming it, made it into a proper sub package, and then renaming it again got back the original name. But this left me with the phantom one I couldn’t delete.

Can you give me the EXACT names you used?

Not sure why you sometimes get those top-level glitches (I’ve seen it a few times).

I will see if I can recreate in 7, or
Maybe this rings some bells?

I'm working on the hypothesis that Announcements changed between Pharo 6.1 and Pharo7, and thus Iceberg's code that listens to system events may be not 100% compatible with Pharo6.1?

@macta
Copy link
Contributor Author

macta commented Aug 1, 2018

We might possibly have the exact sequence in the exercism/Pharo.git repo

Master had:

Exercism
Exercism-HelloWorld

Sam checked in a branch with all top level packages

Exercism
Exercism-HelloWorld
Exercism-TwoFer

I had to delete the master HelloWorld to then merge his branch.

I then did the renaming to sort out the sub packaging issue and now I think about it, I moved some classes from a package called XercismDev to a new one called ExercismDev. I then removed in Pharo the now empty package XercismDev and then it was in the new Iceberg browser where I hit the issue about deleting the XertcismDev package as well as the HelloWorld one I recall.

Anyway it was something like that and it does sound like some announcements failure.

@guillep
Copy link
Member

guillep commented Aug 2, 2018

I've retried to follow your scenario but still, I cannot reproduce THAT issue.
After playing with merges and forcing conflicts from a scenario close to yours, I arrived to the following scenario:

  • I had packages Lala and Lala-Lele
  • I moved all classes from Lala-Lele to Lala
  • Then I removed the package from Nautilus
  • In iceberg, Lala-Lele was (wrongly?) marked as dirty, but the actions that appeared in the contextual menu are correct
  • If I reopen a new iceberg window, it shows that Lala-lele is not loaded

See screenshot:

screen shot 2018-08-02 at 11 15 00

I'm probably doing something differently than you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants