commits disappearing from src-workspace

[rhubert]

We're using commitIDs for most of our packages. This makes the src-workspace be in a deteched HEAD state where it is totally fine to edit and git commit changes. But the changes are not moved to attic if bob is executed with a new commitID for the workspace again.

I extended the git-scm-switch test to demonstrate the issue.

diff --git a/test/git-scm-switch/run.sh b/test/git-scm-switch/run.sh
index 5f9a88b8..37b9ec99 100755
--- a/test/git-scm-switch/run.sh
+++ b/test/git-scm-switch/run.sh
@@ -110,3 +110,22 @@ expect_exist dev/src/root/1/workspace/canary.txt
 run_bob dev -DSCM_DIR="$git_dir1" -DSCM_REV="$d1_c1" root
 expect_not_exist dev/src/root/1/workspace/canary.txt
 expect_output "hello world" cat dev/src/root/1/workspace/test.txt
+
+# make a commit on the detached HEAD we have if we checkout a commit
+run_bob dev -DSCM_DIR="$git_dir1" -DSCM_REV="$d1_c2" root
+pushd dev/src/root/1/workspace
+echo foo >> foo.txt
+git add .
+git commit -m "foo"
+popd
+# a new upstream commit
+pushd "$git_dir1"
+echo "changed2" > test.txt
+git commit -a -m "third commit"
+d1_c3=$(git rev-parse HEAD)
+popd
+# rerun bob with the new upstream commit
+run_bob dev -DSCM_DIR="$git_dir1" -DSCM_REV="$d1_c3" root -vv
+# local commits disappeared...
+pushd dev/src/root/1/
+find . -name "foo.txt" | grep -qz .

Luckily the local commits are still in the git reflog.

[jkloetzke]

This is a side effect of the in-place SCM switching that was added in 0.18. Quoting the release notes:

Most common updates of git SCMs do not trigger a move to attic anymore.

Bob will try to switch the branch/tag/commit without doing a fresh checkout. If this fails, e.g. due to modified files, the checkout will still be moved into the attic and a new checkout will be made. [...]

I have to admit that I never thought about the case where someone would commit to a detached head and then change the commit-id in the recipe. Do we need to add a sanity check that the original commit is still checked out? IMHO this problem is more about expected behaviour. Git even warns about doing commits in this state and actively suggests to create a branch. To be honest I find that move-to-attic approach should always be the last resort. It makes it harder to restore your changes after a recipe change...

[rhubert]

IMO a sanity check + move to attic would be much better than the current behavior. If git symbolic-ref -q HEAD returns 1 and commit != recipe.commit -> move to attic

Maybe we can add a new build argument --no-attic to fail the build instead of moving to attic? After this developer can resolve the conflict in the workspace and run bob again.

[jkloetzke]

IMO a sanity check + move to attic would be much better than the current behavior. If git symbolic-ref -q HEAD returns 1 and commit != recipe.commit -> move to attic

Sounds reasonable. I guess we should always move to attic if the user is not on a branch.

Maybe we can add a new build argument --no-attic to fail the build instead of moving to attic? After this developer can resolve the conflict in the workspace and run bob again.

Interesting idea. Point is, not all SCMs support in-place updates currently. These would always fail with the switch then. But that could be part of the deal...