4 files changed, 142 insertions, 2 deletions
diff --git a/storage/filesystem/dotgit/dotgit.go b/storage/filesystem/dotgit/dotgit.go
index ada51eb..72c9ccf 100644
--- a/storage/filesystem/dotgit/dotgit.go
+++ b/storage/filesystem/dotgit/dotgit.go
@@ -72,6 +72,9 @@ var (
 	// ErrIsDir is returned when a reference file is attempting to be read,
 	// but the path specified is a directory.
 	ErrIsDir = errors.New("reference path is a directory")
+	// ErrEmptyRefFile is returned when a reference file is attempted to be read,
+	// but the file is empty
+	ErrEmptyRefFile = errors.New("ref file is empty")
 )
 
 // Options holds configuration for the storage.
@@ -661,18 +664,33 @@ func (d *DotGit) readReferenceFrom(rd io.Reader, name string) (ref *plumbing.Ref
 		return nil, err
 	}
 
+	if len(b) == 0 {
+		return nil, ErrEmptyRefFile
+	}
+
 	line := strings.TrimSpace(string(b))
 	return plumbing.NewReferenceFromStrings(name, line), nil
 }
 
+// checkReferenceAndTruncate reads the reference from the given file, or the `pack-refs` file if
+// the file was empty. Then it checks that the old reference matches the stored reference and
+// truncates the file.
 func (d *DotGit) checkReferenceAndTruncate(f billy.File, old *plumbing.Reference) error {
 	if old == nil {
 		return nil
 	}
+
 	ref, err := d.readReferenceFrom(f, old.Name().String())
+	if errors.Is(err, ErrEmptyRefFile) {
+		// This may happen if the reference is being read from a newly created file.
+		// In that case, try getting the reference from the packed refs file.
+		ref, err = d.packedRef(old.Name())
+	}
+
 	if err != nil {
 		return err
 	}
+
 	if ref.Hash() != old.Hash() {
 		return storage.ErrReferenceHasChanged
 	}
diff --git a/storage/filesystem/dotgit/dotgit_test.go b/storage/filesystem/dotgit/dotgit_test.go
index 8a5d8dd..fdb8a57 100644
--- a/storage/filesystem/dotgit/dotgit_test.go
+++ b/storage/filesystem/dotgit/dotgit_test.go
@@ -16,6 +16,7 @@ import (
 	"github.com/go-git/go-billy/v5/util"
 	fixtures "github.com/go-git/go-git-fixtures/v4"
 	"github.com/go-git/go-git/v5/plumbing"
+	"github.com/go-git/go-git/v5/storage"
 	"github.com/stretchr/testify/assert"
 	. "gopkg.in/check.v1"
 )
@@ -1046,3 +1047,63 @@ func (s *SuiteDotGit) TestDeletedRefs(c *C) {
 	c.Assert(refs, HasLen, 1)
 	c.Assert(refs[0].Name(), Equals, plumbing.ReferenceName("refs/heads/foo"))
 }
+
+// Checks that seting a reference that has been packed and checking its old value is successful
+func (s *SuiteDotGit) TestSetPackedRef(c *C) {
+	fs, clean := s.TemporalFilesystem()
+	defer clean()
+
+	dir := New(fs)
+
+	err := dir.SetRef(plumbing.NewReferenceFromStrings(
+		"refs/heads/foo",
+		"e8d3ffab552895c19b9fcf7aa264d277cde33881",
+	), nil)
+	c.Assert(err, IsNil)
+
+	refs, err := dir.Refs()
+	c.Assert(err, IsNil)
+	c.Assert(refs, HasLen, 1)
+	looseCount, err := dir.CountLooseRefs()
+	c.Assert(err, IsNil)
+	c.Assert(looseCount, Equals, 1)
+
+	err = dir.PackRefs()
+	c.Assert(err, IsNil)
+
+	// Make sure the refs are still there, but no longer loose.
+	refs, err = dir.Refs()
+	c.Assert(err, IsNil)
+	c.Assert(refs, HasLen, 1)
+	looseCount, err = dir.CountLooseRefs()
+	c.Assert(err, IsNil)
+	c.Assert(looseCount, Equals, 0)
+
+	ref, err := dir.Ref("refs/heads/foo")
+	c.Assert(err, IsNil)
+	c.Assert(ref, NotNil)
+	c.Assert(ref.Hash().String(), Equals, "e8d3ffab552895c19b9fcf7aa264d277cde33881")
+
+	// Attempt to update the reference using an invalid old reference value
+	err = dir.SetRef(plumbing.NewReferenceFromStrings(
+		"refs/heads/foo",
+		"b8d3ffab552895c19b9fcf7aa264d277cde33881",
+	), plumbing.NewReferenceFromStrings(
+		"refs/heads/foo",
+		"e8d3ffab552895c19b9fcf7aa264d277cde33882",
+	))
+	c.Assert(err, Equals, storage.ErrReferenceHasChanged)
+
+	// Now update the reference and it should pass
+	err = dir.SetRef(plumbing.NewReferenceFromStrings(
+		"refs/heads/foo",
+		"b8d3ffab552895c19b9fcf7aa264d277cde33881",
+	), plumbing.NewReferenceFromStrings(
+		"refs/heads/foo",
+		"e8d3ffab552895c19b9fcf7aa264d277cde33881",
+	))
+	c.Assert(err, IsNil)
+	looseCount, err = dir.CountLooseRefs()
+	c.Assert(err, IsNil)
+	c.Assert(looseCount, Equals, 1)
+}
diff --git a/storage/filesystem/object.go b/storage/filesystem/object.go
index e812fe9..91b4ace 100644
--- a/storage/filesystem/object.go
+++ b/storage/filesystem/object.go
@@ -431,13 +431,13 @@ func (s *ObjectStorage) getFromUnpacked(h plumbing.Hash) (obj plumbing.EncodedOb
 
 	defer ioutil.CheckClose(w, &err)
 
-	s.objectCache.Put(obj)
-
 	bufp := copyBufferPool.Get().(*[]byte)
 	buf := *bufp
 	_, err = io.CopyBuffer(w, r, buf)
 	copyBufferPool.Put(bufp)
 
+	s.objectCache.Put(obj)
+
 	return obj, err
 }
 
diff --git a/storage/filesystem/object_test.go b/storage/filesystem/object_test.go
index 251077a..4f98458 100644
--- a/storage/filesystem/object_test.go
+++ b/storage/filesystem/object_test.go
@@ -547,3 +547,64 @@ func BenchmarkGetObjectFromPackfile(b *testing.B) {
 		})
 	}
 }
+
+func (s *FsSuite) TestGetFromUnpackedCachesObjects(c *C) {
+	fs := fixtures.ByTag(".git").ByTag("unpacked").One().DotGit()
+	objectCache := cache.NewObjectLRUDefault()
+	objectStorage := NewObjectStorage(dotgit.New(fs), objectCache)
+	hash := plumbing.NewHash("f3dfe29d268303fc6e1bbce268605fc99573406e")
+
+	// Assert the cache is empty initially
+	_, ok := objectCache.Get(hash)
+	c.Assert(ok, Equals, false)
+
+	// Load the object
+	obj, err := objectStorage.EncodedObject(plumbing.AnyObject, hash)
+	c.Assert(err, IsNil)
+	c.Assert(obj.Hash(), Equals, hash)
+
+	// The object should've been cached during the load
+	cachedObj, ok := objectCache.Get(hash)
+	c.Assert(ok, Equals, true)
+	c.Assert(cachedObj, DeepEquals, obj)
+
+	// Assert that both objects can be read and that they both produce the same bytes
+
+	objReader, err := obj.Reader()
+	c.Assert(err, IsNil)
+	objBytes, err := io.ReadAll(objReader)
+	c.Assert(err, IsNil)
+	c.Assert(len(objBytes), Not(Equals), 0)
+	err = objReader.Close()
+	c.Assert(err, IsNil)
+
+	cachedObjReader, err := cachedObj.Reader()
+	c.Assert(err, IsNil)
+	cachedObjBytes, err := io.ReadAll(cachedObjReader)
+	c.Assert(len(cachedObjBytes), Not(Equals), 0)
+	c.Assert(err, IsNil)
+	err = cachedObjReader.Close()
+	c.Assert(err, IsNil)
+
+	c.Assert(cachedObjBytes, DeepEquals, objBytes)
+}
+
+func (s *FsSuite) TestGetFromUnpackedDoesNotCacheLargeObjects(c *C) {
+	fs := fixtures.ByTag(".git").ByTag("unpacked").One().DotGit()
+	objectCache := cache.NewObjectLRUDefault()
+	objectStorage := NewObjectStorageWithOptions(dotgit.New(fs), objectCache, Options{LargeObjectThreshold: 1})
+	hash := plumbing.NewHash("f3dfe29d268303fc6e1bbce268605fc99573406e")
+
+	// Assert the cache is empty initially
+	_, ok := objectCache.Get(hash)
+	c.Assert(ok, Equals, false)
+
+	// Load the object
+	obj, err := objectStorage.EncodedObject(plumbing.AnyObject, hash)
+	c.Assert(err, IsNil)
+	c.Assert(obj.Hash(), Equals, hash)
+
+	// The object should not have been cached during the load
+	_, ok = objectCache.Get(hash)
+	c.Assert(ok, Equals, false)
+}