Loading a branch that moves classes from one package to another loses those classes

[macta]

I wanted to test a PR on our project and couldn't easily do it in iceberg and github said:

Step 1: From your project repository, check out a new branch and test the changes.

 git checkout -b jaccarmac-space-age master
git pull https://github.com/jaccarmac/pharo-smalltalk.git space-age

Step 2: Merge the changes and update on GitHub.

Step 1 I did in the command line - and it worked a charm (source tree confirmed it too).

So then I needed to update my image (to match the command line merge) - in pharo it was showing master and detached head. So I chose to repair and pick that branch - for a moment it looked right but when I chose commit - it showed all the new changes as being removals and not additions?

I tired to swap back to master and then checkout that branch again to make it show correctly (I have noticed this in the past but this case is actually different).

It didn't work - as when I pick the branch I created - the preview shows correctly with a SpaceAge class and SpaceAgeTest, along with the removal of an existing SpaceAgeTest in a different package.

When done there is no SpaceAgeTest anymore. It doesn't understand that package move (so i end up with the last operation being SpaceAgeTest being deleted).

Interestingly - Epicea shows the change as AnObsoleteSpaceAgeTest with many method additions to it (and so not visible in the image).

[macta]

I have kept this image if its useful - but I think you can see whats happened.

We have a ExercismWIP package, that has tags with tests that people can fill out and then move to the main Exercism package along with a sample solution. It seems that this muddles up Iceberg?

[macta]

Just for the record, this is what I did at the command line (as per github's instructions)

Tims-MacBook-Pro:pharo-exercism macta$ git pull https://github.com/jaccarmac/pharo-smalltalk.git space-age
remote: Enumerating objects: 30, done.
remote: Counting objects: 100% (30/30), done.
remote: Compressing objects: 100% (20/20), done.
remote: Total 30 (delta 15), reused 25 (delta 10), pack-reused 0
Unpacking objects: 100% (30/30), done.
From https://github.com/jaccarmac/pharo-smalltalk
 * branch            space-age  -> FETCH_HEAD
Auto-merging dev/src/ExercismTools/ExercismTest.class.st
Auto-merging dev/src/Exercism/SpaceAgeTest.class.st
Merge made by the 'recursive' strategy.
 dev/src/Exercism/SpaceAge.class.st                 | 32 +++++++++++
 .../SpaceAgeTest.class.st                          | 64 +++++++++++++---------
 dev/src/ExercismTools/ExercismTest.class.st        |  9 +++
 3 files changed, 79 insertions(+), 26 deletions(-)
 create mode 100644 dev/src/Exercism/SpaceAge.class.st
 rename dev/src/{ExercismWIP => Exercism}/SpaceAgeTest.class.st (52%)
Tims-MacBook-Pro:pharo-exercism macta$ 

[macta]

I've encountered this one again - its a bit of a pain

[macta]

I can give another clearer example of this now -

I pulled from upstream on master and had my fork and upstream all nicely in sync. I then worked on two more exercism exercises - and so moved 2 TestCases from a WIP package to my mainline Exercism package and then I created solutions for them. All great - very clean.

I then created a branch (anagram-isogram-exercises) and then commited and pushed to my fork. I created a PR, noticed a bug, made a fix and pushed again. I was happy and everything working well.

Now I wanted to work on a new exercise (but keep it seperate for easy review). So I switched back to master, pulled from upstream again and then discovered the anagram and isogram tests had disappeared from my image. Looking at Epicea I can see changes to "AnObsoleteIsogramTest".

so this is a simple and repeatable test case now. (there are also lots of exercises in exercism, so you can easily load it, and repeat the steps above to see for yourself)

[macta]

The workaround is to right-click "Reload" the package from the working copy window and not merge. However its not ideal as you have to know this

[macta]

To reproduce we can take commit ff1068f and merge 06e6b30. The merge should not have moved the TwelveDays class nor the test, but it has moved TwelveDays

[macta]

Also, another similar issue can be reproduced by checking out first macta/checkout-setup-issue1208 and then macta/checkout-to-break-issue1208. Then Iceberg loses the class FoodChainTest. Moreover it does not mark the package as dirty unless we recalculate it explicitly

[guillep]

I've managed to reproduce it and manage the cause :).

The point is that between the two commits the FoodChainTest class was moved from WIP to the main package. When Iceberg loads this, it first creates a Diff tree with:

• MainPackage / FoodChainTest (addition)
• WIPPackage / FoodChainTest (removal)

And then we create a Monticello patch with all the changes, so the patch contains:

• adding the class
• then removing the class

So it's like the patch is cancelling itself. I'll build a test and see how to fix it now.

Thanks!!

[macta]

No problem - thanks for sticking with it, it’s definitely the way forward (and drags us all kicking and screaming into the mainstream)

[macta]

In case it helps - I have another example - a user contributed an exercise - git://github.com/carlotxra/pharo-smalltalk.git in branch issue-272. I load this branch and one of his classes disappears (the Test one again). I think the commitish is: 3622b2b934d6747d40fb2b9137e46229407f025d: but you can see it in this photo - the diff shows both classes, but in my image the Test one is missing. (Epicea shows it as an ObsoleteTrinaryTest)