[BUG] Ensure a git worktree cleans up after itself

[oliv3r]

As mentioned in #555, using the current docker kicad 7 full container, when we create the worktree's, we are not guaranteed to checkout the correct sha.

When one has a git repository, with several branches, a branch can be 'outdated' locally. To update such a branch, one normally checks out the branch, and then updates it.

Creating a worktree, just checks out a branch, but never updates it. So if I have pushed an update to, for example 'dev' from a nother system, and I tell kibot to diff a branch, which branch should it use? The remote or the local branch. On a local system, Its more then natural to do exactly what the user requested and use the local branch. After all, the user said 'compare to branch', and they surely would use remotes/origin/dev if they wanted to compare to upstream.

So maybe this is a gitlab bug? What gitlab does, is they fetch the repository, and then do a detached checkout. They do this because they can cache the git repository itself, and re-use it amonst several jobs etc. But then, where does the branch even come from? It must be a remanant of kibot creating a worktree, but not cleaned up. E.g. git worktree will checkout a (remote) branch, which creates a local branch, but it will not remove the branch with git worktree remove!

[oliver@2of9:~]% git worktree add /tmp/test test
Preparing worktree (new branch 'test')
branch 'test' set up to track 'origin/test'.
HEAD is now at aec0a52 test
[oliver@2of9:~/src/ci-includes/masslinter/kicad]% git branch
* dev
+ test
[oliver@2of9:~/src/ci-includes/masslinter/kicad]% git worktree remove /tmp/test
[oliver@2of9:~/src/ci-includes/masslinter/kicad]% git branch
* dev
  test

The tricky bit now is, we do not know if the branch already existed! E.g the above flow is logical and sensible, with the context that there was no branch checked out. However, on a local system, how annoying it may be in some cases, the user may want exactly that, against the requested branch, so we cannot do an 'always remove', but instead 'only remove if it did not exist before', kind of thing.

One thing that is interesting to note, in the above, if the branch does exist locally, git will create a worktree (in detached head mode), but it won't show up with the '+' in git branch. Git worktree does something different itself as well, in the 'if the branch exists locally, and has the same hash, use the branch, if it exists locally, but doesn't have the same hash, detached worktree, if it doesn't exist, create a local branch'.

As a work-around, I'm currently running git branch -D $(git branch -l | grep -v "^*") at the end of the pipeline to remove any stale branches, but that's far far from clean and ideal.

Doing a little more investigation, I notice that there's a switch actually to force detached mode, --detach. Further more, I realized that git worktree will not always checkout the remote branch!

git worktree add --detach /tmp/master master
fatal: invalid reference: master

Which was a bit of a supprise.

So I think, kibot should always use --detach mode, so that it doesn't create a branch, that we need to clean.
As for the commitish, I'll just have to always use origin/<branch> in my script. It sucks on one hand, because the remote may not always be called origin; though in the case of gitlab, it's safe to assume so, but that's out of scope for this bug.

[set-soft]

I'm not sure I'm understanding your problem:

1. The state of the repo before running KiBot is responsibility of the user. If GitLab does a partial checkout you must fix it. In the GitHub case the checkout action does a shallow checkout by default, so you must explicitly ask for a deeper checkout.
2. KiBot should leave the repo without undesired changes. This is why we always remove all the created worktrees (providing the unique name used for each worktree).

Now let me see if I understand correctly: not using --detach could result in an undesirable synchronization from the remote, is that your problem?

Adding --detach seems to be ok for what we need, we won't modify the worktree.

[oliv3r]

I'm not sure I'm understanding your problem:

1. The state of the repo before running KiBot is responsibility of the user. If GitLab does a partial checkout you must fix it. In the GitHub case the checkout action does a shallow checkout by default, so you must explicitly ask for a deeper checkout.

Correct, which is why I have setup GIT_DEPTH: 0 to ensure that gitlab always fetches everything.

However, if a checked out branch exists in the repo, that will of course not be updated, only the 'remote' version of said branch. E.g. dev was in my case still pointing to something old, wheras origin/dev is what I was interested in, as I pushed new stuff.

Who put that branch there? Well not gitlab, as gitlab only does a detached checkout of a 'bare' repo. So 'someone' is running git commands that change/influence the repo. Since I'm only using this repo with kibot, that must be it?

2. KiBot should leave the repo without undesired changes. This is why we always remove all the created worktrees (providing the unique name used for each worktree).

It does cleanup, but as you can see from my examples, it leaves a branch behind, which is not nice. Hence my patch to use detached mode, which should avoid creating a branch altogether.

Now let me see if I understand correctly: not using --detach could result in an undesirable synchronization from the remote, is that your problem?
Exactly, I think this synchronization issue is what's causing the problem

Adding --detach seems to be ok for what we need, we won't modify the worktree.

I would think so too, I just did a patch, I'm now patching kibot in the CI to see if this works as expected.

[set-soft]

Ok

[oliv3r]

Going to use the dev container now that you have merged it to test it:) thanks.