Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 101c1ab

Browse filesBrowse files
Javier-varezJavier-varez
committed
storage: Fix reference updated concurrently error for the filesystem storer
If a reference exists in the packed refs file but not in the actual refs folder, we need to fall back to reading the pack refs file when storing the reference. This happens because by the time we are storing the reference, we open the file with write permissions and read from it. At that point, if the file is empty, we still need to read the valid reference. Otherwise, it will read 0, and assume that the references do not match (because the old value was read from the packed file).
1 parent 1e72372 commit 101c1ab
Copy full SHA for 101c1ab

File tree

Expand file treeCollapse file tree

2 files changed

+79
-0
lines changed
Filter options
Expand file treeCollapse file tree

2 files changed

+79
-0
lines changed

‎storage/filesystem/dotgit/dotgit.go

Copy file name to clipboardExpand all lines: storage/filesystem/dotgit/dotgit.go
+18Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,9 @@ var (
7272
// ErrIsDir is returned when a reference file is attempting to be read,
7373
// but the path specified is a directory.
7474
ErrIsDir = errors.New("reference path is a directory")
75+
// ErrEmptyRefFile is returned when a reference file is attempted to be read,
76+
// but the file is empty
77+
ErrEmptyRefFile = errors.New("ref file is empty")
7578
)
7679

7780
// Options holds configuration for the storage.
@@ -661,18 +664,33 @@ func (d *DotGit) readReferenceFrom(rd io.Reader, name string) (ref *plumbing.Ref
661664
return nil, err
662665
}
663666

667+
if len(b) == 0 {
668+
return nil, ErrEmptyRefFile
669+
}
670+
664671
line := strings.TrimSpace(string(b))
665672
return plumbing.NewReferenceFromStrings(name, line), nil
666673
}
667674

675+
// checkReferenceAndTruncate reads the reference from the given file, or the `pack-refs` file if
676+
// the file was empty. Then it checks that the old reference matches the stored reference and
677+
// truncates the file.
668678
func (d *DotGit) checkReferenceAndTruncate(f billy.File, old *plumbing.Reference) error {
669679
if old == nil {
670680
return nil
671681
}
682+
672683
ref, err := d.readReferenceFrom(f, old.Name().String())
684+
if errors.Is(err, ErrEmptyRefFile) {
685+
// This may happen if the reference is being read from a newly created file.
686+
// In that case, try getting the reference from the packed refs file.
687+
ref, err = d.packedRef(old.Name())
688+
}
689+
673690
if err != nil {
674691
return err
675692
}
693+
676694
if ref.Hash() != old.Hash() {
677695
return storage.ErrReferenceHasChanged
678696
}

‎storage/filesystem/dotgit/dotgit_test.go

Copy file name to clipboardExpand all lines: storage/filesystem/dotgit/dotgit_test.go
+61Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/go-git/go-billy/v5/util"
1717
fixtures "github.com/go-git/go-git-fixtures/v4"
1818
"github.com/go-git/go-git/v5/plumbing"
19+
"github.com/go-git/go-git/v5/storage"
1920
"github.com/stretchr/testify/assert"
2021
. "gopkg.in/check.v1"
2122
)
@@ -1046,3 +1047,63 @@ func (s *SuiteDotGit) TestDeletedRefs(c *C) {
10461047
c.Assert(refs, HasLen, 1)
10471048
c.Assert(refs[0].Name(), Equals, plumbing.ReferenceName("refs/heads/foo"))
10481049
}
1050+
1051+
// Checks that seting a reference that has been packed and checking its old value is successful
1052+
func (s *SuiteDotGit) TestSetPackedRef(c *C) {
1053+
fs, clean := s.TemporalFilesystem()
1054+
defer clean()
1055+
1056+
dir := New(fs)
1057+
1058+
err := dir.SetRef(plumbing.NewReferenceFromStrings(
1059+
"refs/heads/foo",
1060+
"e8d3ffab552895c19b9fcf7aa264d277cde33881",
1061+
), nil)
1062+
c.Assert(err, IsNil)
1063+
1064+
refs, err := dir.Refs()
1065+
c.Assert(err, IsNil)
1066+
c.Assert(refs, HasLen, 1)
1067+
looseCount, err := dir.CountLooseRefs()
1068+
c.Assert(err, IsNil)
1069+
c.Assert(looseCount, Equals, 1)
1070+
1071+
err = dir.PackRefs()
1072+
c.Assert(err, IsNil)
1073+
1074+
// Make sure the refs are still there, but no longer loose.
1075+
refs, err = dir.Refs()
1076+
c.Assert(err, IsNil)
1077+
c.Assert(refs, HasLen, 1)
1078+
looseCount, err = dir.CountLooseRefs()
1079+
c.Assert(err, IsNil)
1080+
c.Assert(looseCount, Equals, 0)
1081+
1082+
ref, err := dir.Ref("refs/heads/foo")
1083+
c.Assert(err, IsNil)
1084+
c.Assert(ref, NotNil)
1085+
c.Assert(ref.Hash().String(), Equals, "e8d3ffab552895c19b9fcf7aa264d277cde33881")
1086+
1087+
// Attempt to update the reference using an invalid old reference value
1088+
err = dir.SetRef(plumbing.NewReferenceFromStrings(
1089+
"refs/heads/foo",
1090+
"b8d3ffab552895c19b9fcf7aa264d277cde33881",
1091+
), plumbing.NewReferenceFromStrings(
1092+
"refs/heads/foo",
1093+
"e8d3ffab552895c19b9fcf7aa264d277cde33882",
1094+
))
1095+
c.Assert(err, Equals, storage.ErrReferenceHasChanged)
1096+
1097+
// Now update the reference and it should pass
1098+
err = dir.SetRef(plumbing.NewReferenceFromStrings(
1099+
"refs/heads/foo",
1100+
"b8d3ffab552895c19b9fcf7aa264d277cde33881",
1101+
), plumbing.NewReferenceFromStrings(
1102+
"refs/heads/foo",
1103+
"e8d3ffab552895c19b9fcf7aa264d277cde33881",
1104+
))
1105+
c.Assert(err, IsNil)
1106+
looseCount, err = dir.CountLooseRefs()
1107+
c.Assert(err, IsNil)
1108+
c.Assert(looseCount, Equals, 1)
1109+
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.