hpi-swa / Squot

Squeak Object Tracker - Version control for arbitrary objects, currently with Git storage
Other
61 stars 29 forks source link

"Switch to this branch, but keep uncommited changes" discards changes from previously untracked packages #364

Closed LinqLover closed 2 years ago

LinqLover commented 2 years ago

Let branch a contain a package P that is not contained in branch b. After switching to branch b, the package P is kept in the image. If the user modifies the - now untracked - package P and then chooses "switch to this branch (a), but keep uncomitted changes", the modifications to package P are lost!

LinqLover commented 2 years ago

I was able to work around the issue by using the undocumented "make this the current branch" feature for branch b.

LinqLover commented 2 years ago

Unfortunately, the workaround is not applicable for pulling from the remote.

j4yk commented 2 years ago

Reproduced

j4yk commented 2 years ago

What happens is the following: During the switch, first the unsaved changes are saved. Since the package is not tracked at the moment, it is not saved here. Then Squot proceeds to load (as in: overwrite) the destination version into the working copy. When it does that, it first adds all packages to the working copy, which are loaded and tracked in the destination version, so the package gets added to the working copy here. Then it proceeds to compute the patch to transform the package into the state at the destination version, and loads that patch non-interactively, so you lose your unsaved changes to the package. Afterwards it first restores all unsaved changes from the destination branch, and then it applies your previously saved "unsaved changes" from the working copy. But as noted before, the latter does not include the then-untracked package, so it does not get restored.

So this switch procedure must be changed to look a bit further ahead and also consider packages that will be added during the switch and that are already loaded in the image. It should make a snapshot of such packages and must then, unfortunately, do yet another merge step during the switch to keep such packages up to date. Question is, what should be assumed as the base of the three-way merge of such a package? With none (as in: the branch one switches away from, since it does not have the package), all modifications made to the package will show as conflicts. With the snapshot of the package before the switch as the base, it would look like the destination branch has modified the package, while there were no changes to it on the branch which actually does not contain the package. With the destination branch as the base, all modifications will appear as modifications to apply the uncommitted changes to the destination branch. The latter sounds the most useful to me, but unfortunately that does not follow the logic of cherry-picking (where the predecessor of the commit to pick is used as the virtual base of the merge), so I will have to put more thought into the consequences.

j4yk commented 2 years ago

@LinqLover Please try whether it works as expected for you now.

LinqLover commented 2 years ago

Sorry for the late reply ... Let's work through the initial repocase again (from my naive perspective without your deep understanding of Squot's internals :D):

Let branch a contain a package P that is not contained in branch b. After switching to branch b, the package P is kept in the image.

With the latest change, the switch now does the following:

So eventually, this all looks fine to me.

If the user modifies the - now untracked - package P and then chooses "switch to this branch (a), but keep uncomitted changes", the modifications to package P are lost!

All of this "dark path" is no longer available (unless I use the undocumented "make this the current branch" feature in the previous step to keep P, in which case the new modifications will still be lost - but in this case, I confess that its my fault and not the system's fault :D).

tl;dr: Thank you for fixing this! Looks and feels all like it should now.