Attempting to check out a non-existent commit stores the bad SHA-1 in HEAD

[magnusbaeck]

If you attempt to check out a non-existent commit SHA-1 that operation fails, unsurprisingly, with plumbing.ErrObjectNotFound. What's surprising is that the SHA-1 remains in .git/HEAD. This isn't consistent with cgit's behavior and can result in some really confusing errors. Here's a minimal example:

package main

import (
	"path/filepath"

	"github.com/go-git/go-billy/v5/osfs"
	"github.com/go-git/go-git/v5"
	"github.com/go-git/go-git/v5/plumbing"
	"github.com/go-git/go-git/v5/plumbing/cache"
	"github.com/go-git/go-git/v5/storage/filesystem"
)

const gitPath = "./git"

func main() {
	worktreeFS := osfs.New(gitPath)
	gitDirFS := osfs.New(filepath.Join(gitPath, ".git"))
	repo, err := git.Init(filesystem.NewStorage(gitDirFS, cache.NewObjectLRUDefault()), worktreeFS)
	if err != nil {
		panic(err)
	}
	tree, err := repo.Worktree()
	if err != nil {
		panic(err)
	}
	err = tree.Checkout(
		&git.CheckoutOptions{
			Hash:  plumbing.NewHash("1dafc1f4067af82145a8fb8248b11df5634dc3e8"), // Any random SHA-1
			Force: true,
		})
	if err != nil {
		panic(err)
	}
}

Result after running this:

$ cd git
$ cat .git/HEAD 
1dafc1f4067af82145a8fb8248b11df5634dc3e8
$ git status
fatal: bad object HEAD

I discovered this when writing code that speculatively checked out a commit that might be present locally. When that failed with plumbing.ErrObjectNotFound I fetched a ref that I knew pointed to the desired commit. However, because HEAD pointed to the commit in question the SHA-1 was listed not only in the wants set of the fetch operation but also the haves set, so the fetch operation failed with transport.ErrEmptyUploadPackRequest.

It looks like this would be fairly easy to address by simply verifying that opts.Hash resolves to something that can be checked out prior to this code in Worktree.Checkout:

go-git/worktree.go

Lines 175 to 179 in f92011d

	if !opts.Hash.IsZero() && !opts.Create {
	err = w.setHEADToCommit(opts.Hash)
	} else {
	err = w.setHEADToBranch(opts.Branch, c)
	}

[github-actions]

To help us keep things tidy and focus on the active tasks, we've introduced a stale bot to spot issues/PRs that haven't had any activity in a while.

This particular issue hasn't had any updates or activity in the past 90 days, so it's been labeled as 'stale'. If it remains inactive for the next 30 days, it'll be automatically closed.

We understand everyone's busy, but if this issue is still important to you, please feel free to add a comment or make an update to keep it active.

Thanks for your understanding and cooperation!

[onee-only]

I think it was fixed in #966.
Tested with latest release and it works as expected.

[pjbgf]

@onee-only nice one, thank you.

Closing as per above.