Inspect changes in Gitaly 0c4699284cdf6a385bfd4383776b35bd1c3a8f0e
Upstream merge commit c1ef53276082 changes Gitaly server version from b2a3b6ba03e6f2c2ad60582733fd27f050d8aa3f to 0c4699284cdf6a385bfd4383776b35bd1c3a8f0e, with changes in protocol or one of the source subtrees currently under watch.
To further inspect, go to an up to date Gitaly source clone and run
diff b2a3b6ba03e6f2c2ad60582733fd27f050d8aa3f..0c4699284cdf6a385bfd4383776b35bd1c3a8f0e -- proto/*.proto internal/backup
. Here is a preview, truncated at 500 lines:
diff --git a/internal/backup/backup.go b/internal/backup/backup.go
index b2a594992..fd308abd2 100644
--- a/internal/backup/backup.go
+++ b/internal/backup/backup.go
@@ -49,10 +49,6 @@ type Backup struct {
type Step struct {
// BundlePath is the path of the bundle
BundlePath string
- // SkippableOnNotFound defines if the bundle can be skipped when it does
- // not exist. This allows us to maintain legacy behaviour where we always
- // check a specific location for a bundle without knowing if it exists.
- SkippableOnNotFound bool
// RefPath is the path of the ref file
RefPath string
// PreviousRefPath is the path of the previous ref file
@@ -274,23 +270,33 @@ func (mgr *Manager) Restore(ctx context.Context, req *RestoreRequest) error {
}
for _, step := range backup.Steps {
- if err := mgr.restoreBundle(ctx, repo, step.BundlePath); err != nil {
- if step.SkippableOnNotFound && errors.Is(err, ErrDoesntExist) {
- // For compatibility with existing backups we need to make sure the
- // repository exists even if there's no bundle for project
- // repositories (not wiki or snippet repositories). Gitaly does
- // not know which repository is which type so here we accept a
- // parameter to tell us to employ this behaviour. Since the
- // repository has already been created, we simply skip cleaning up.
- if req.AlwaysCreate {
- return nil
- }
-
- if err := repo.Remove(ctx); err != nil {
- return fmt.Errorf("manager: remove on skipped: %w", err)
- }
-
- return fmt.Errorf("manager: %w: %s", ErrSkipped, err.Error())
+ refs, err := mgr.readRefs(ctx, step.RefPath)
+ switch {
+ case errors.Is(err, ErrDoesntExist):
+ // For compatibility with existing backups we need to make sure the
+ // repository exists even if there's no bundle for project
+ // repositories (not wiki or snippet repositories). Gitaly does
+ // not know which repository is which type so here we accept a
+ // parameter to tell us to employ this behaviour. Since the
+ // repository has already been created, we simply skip cleaning up.
+ if req.AlwaysCreate {
+ return nil
+ }
+
+ if err := repo.Remove(ctx); err != nil {
+ return fmt.Errorf("manager: remove on skipped: %w", err)
+ }
+
+ return fmt.Errorf("manager: %w: %s", ErrSkipped, err.Error())
+ case err != nil:
+ return fmt.Errorf("manager: %w", err)
+ }
+
+ // Git bundles can not be created for empty repositories. Since empty
+ // repository backups do not contain a bundle, skip bundle restoration.
+ if len(refs) > 0 {
+ if err := mgr.restoreBundle(ctx, repo, step.BundlePath); err != nil {
+ return fmt.Errorf("manager: %w", err)
}
}
if err := mgr.restoreCustomHooks(ctx, repo, step.CustomHooksPath); err != nil {
@@ -405,6 +411,31 @@ func (mgr *Manager) negatedKnownRefs(ctx context.Context, step *Step) (io.ReadCl
return r, nil
}
+func (mgr *Manager) readRefs(ctx context.Context, path string) ([]git.Reference, error) {
+ reader, err := mgr.sink.GetReader(ctx, path)
+ if err != nil {
+ return nil, fmt.Errorf("read refs: %w", err)
+ }
+ defer reader.Close()
+
+ var refs []git.Reference
+
+ d := git.NewShowRefDecoder(reader)
+ for {
+ var ref git.Reference
+
+ if err := d.Decode(&ref); err == io.EOF {
+ break
+ } else if err != nil {
+ return refs, fmt.Errorf("read refs: %w", err)
+ }
+
+ refs = append(refs, ref)
+ }
+
+ return refs, nil
+}
+
func (mgr *Manager) restoreBundle(ctx context.Context, repo Repository, path string) error {
reader, err := mgr.sink.GetReader(ctx, path)
if err != nil {
diff --git a/internal/backup/backup_test.go b/internal/backup/backup_test.go
index af905e9d0..f437ce6a5 100644
--- a/internal/backup/backup_test.go
+++ b/internal/backup/backup_test.go
@@ -433,6 +433,8 @@ func TestManager_Restore_latest(t *testing.T) {
commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"))
gittest.WriteTag(t, cfg, repoPath, "v1.0.0", commitID.Revision())
repoChecksum := gittest.ChecksumRepo(t, cfg, repoPath)
+ repoBundle := gittest.BundleRepo(t, cfg, repoPath, "-")
+ repoRefs := gittest.Exec(t, cfg, "-C", repoPath, "show-ref", "--head")
backupRoot := testhelper.TempDir(t)
@@ -452,9 +454,10 @@ func TestManager_Restore_latest(t *testing.T) {
repo, _ := gittest.CreateRepository(t, ctx, cfg)
relativePath := stripRelativePath(tb, repo)
- require.NoError(tb, os.MkdirAll(filepath.Join(backupRoot, relativePath), perm.PublicDir))
- bundlePath := filepath.Join(backupRoot, relativePath+".bundle")
- gittest.BundleRepo(tb, cfg, repoPath, bundlePath)
+ testhelper.WriteFiles(tb, backupRoot, map[string]any{
+ relativePath + ".bundle": repoBundle,
+ relativePath + ".refs": repoRefs,
+ })
return repo, repoChecksum
},
@@ -467,10 +470,13 @@ func TestManager_Restore_latest(t *testing.T) {
repo, _ := gittest.CreateRepository(t, ctx, cfg)
relativePath := stripRelativePath(tb, repo)
- bundlePath := filepath.Join(backupRoot, relativePath+".bundle")
customHooksPath := filepath.Join(backupRoot, relativePath, "custom_hooks.tar")
+ testhelper.WriteFiles(tb, backupRoot, map[string]any{
+ relativePath + ".bundle": repoBundle,
+ relativePath + ".refs": repoRefs,
+ })
+
require.NoError(tb, os.MkdirAll(filepath.Join(backupRoot, relativePath), perm.PublicDir))
- gittest.BundleRepo(tb, cfg, repoPath, bundlePath)
testhelper.CopyFile(tb, mustCreateCustomHooksArchive(t, ctx), customHooksPath)
return repo, repoChecksum
@@ -508,13 +514,13 @@ func TestManager_Restore_latest(t *testing.T) {
repo, _ := gittest.CreateRepository(t, ctx, cfg)
relativePath := stripRelativePath(tb, repo)
- refsPath := filepath.Join(backupRoot, relativePath+".refs")
- require.NoError(tb, os.MkdirAll(filepath.Join(backupRoot, relativePath), perm.PublicDir))
- require.NoError(tb, os.WriteFile(refsPath, []byte{}, perm.SharedFile))
+ testhelper.WriteFiles(tb, backupRoot, map[string]any{
+ relativePath + ".refs": "",
+ })
- return repo, nil
+ return repo, new(git.Checksum)
},
- expectedErrAs: backup.ErrSkipped,
+ expectExists: true,
},
{
desc: "empty backup, always create",
@@ -523,11 +529,11 @@ func TestManager_Restore_latest(t *testing.T) {
repo, _ := gittest.CreateRepository(t, ctx, cfg)
relativePath := stripRelativePath(tb, repo)
- refsPath := filepath.Join(backupRoot, relativePath+".refs")
- require.NoError(tb, os.MkdirAll(filepath.Join(backupRoot, relativePath), perm.PublicDir))
- require.NoError(tb, os.WriteFile(refsPath, []byte{}, perm.SharedFile))
+ testhelper.WriteFiles(tb, backupRoot, map[string]any{
+ relativePath + ".refs": "",
+ })
- return repo, nil
+ return repo, new(git.Checksum)
},
alwaysCreate: true,
expectExists: true,
@@ -542,9 +548,10 @@ func TestManager_Restore_latest(t *testing.T) {
}
relativePath := stripRelativePath(tb, repo)
- require.NoError(tb, os.MkdirAll(filepath.Dir(filepath.Join(backupRoot, relativePath)), perm.PublicDir))
- bundlePath := filepath.Join(backupRoot, relativePath+".bundle")
- gittest.BundleRepo(tb, cfg, repoPath, bundlePath)
+ testhelper.WriteFiles(tb, backupRoot, map[string]any{
+ relativePath + ".bundle": repoBundle,
+ relativePath + ".refs": repoRefs,
+ })
return repo, repoChecksum
},
@@ -556,13 +563,14 @@ func TestManager_Restore_latest(t *testing.T) {
setup: func(tb testing.TB) (*gitalypb.Repository, *git.Checksum) {
const backupID = "abc123"
repo, _ := gittest.CreateRepository(t, ctx, cfg)
- repoBackupPath := joinBackupPath(tb, backupRoot, repo)
- backupPath := filepath.Join(repoBackupPath, backupID)
- require.NoError(tb, os.MkdirAll(backupPath, perm.PublicDir))
- require.NoError(tb, os.WriteFile(filepath.Join(repoBackupPath, "LATEST"), []byte(backupID), perm.PublicFile))
- require.NoError(tb, os.WriteFile(filepath.Join(backupPath, "LATEST"), []byte("001"), perm.PublicFile))
- bundlePath := filepath.Join(backupPath, "001.bundle")
- gittest.BundleRepo(tb, cfg, repoPath, bundlePath)
+
+ relativePath := stripRelativePath(tb, repo)
+ testhelper.WriteFiles(tb, backupRoot, map[string]any{
+ filepath.Join(relativePath, "LATEST"): backupID,
+ filepath.Join(relativePath, backupID, "LATEST"): "001",
+ filepath.Join(relativePath, backupID, "001.bundle"): repoBundle,
+ filepath.Join(relativePath, backupID, "001.refs"): repoRefs,
+ })
return repo, repoChecksum
},
@@ -574,13 +582,13 @@ func TestManager_Restore_latest(t *testing.T) {
setup: func(tb testing.TB) (*gitalypb.Repository, *git.Checksum) {
const backupID = "abc123"
repo, _ := gittest.CreateRepository(t, ctx, cfg)
- repoBackupPath := joinBackupPath(tb, backupRoot, repo)
- backupPath := filepath.Join(repoBackupPath, backupID)
- require.NoError(tb, os.MkdirAll(backupPath, perm.PublicDir))
- require.NoError(tb, os.WriteFile(filepath.Join(repoBackupPath, "LATEST"), []byte(backupID), perm.PublicFile))
- require.NoError(tb, os.WriteFile(filepath.Join(backupPath, "LATEST"), []byte("001"), perm.PublicFile))
- refsPath := filepath.Join(backupPath, "001.refs")
- require.NoError(tb, os.WriteFile(refsPath, []byte{}, perm.SharedFile))
+
+ relativePath := stripRelativePath(tb, repo)
+ testhelper.WriteFiles(tb, backupRoot, map[string]any{
+ filepath.Join(relativePath, "LATEST"): backupID,
+ filepath.Join(relativePath, backupID, "LATEST"): "001",
+ filepath.Join(relativePath, backupID, "001.refs"): "",
+ })
return repo, new(git.Checksum)
},
@@ -595,11 +603,6 @@ func TestManager_Restore_latest(t *testing.T) {
_, expectedRepoPath := gittest.CreateRepository(t, ctx, cfg)
repo, _ := gittest.CreateRepository(t, ctx, cfg)
- repoBackupPath := joinBackupPath(tb, backupRoot, repo)
- backupPath := filepath.Join(repoBackupPath, backupID)
- require.NoError(tb, os.MkdirAll(backupPath, perm.PublicDir))
- require.NoError(tb, os.WriteFile(filepath.Join(repoBackupPath, "LATEST"), []byte(backupID), perm.PublicFile))
- require.NoError(tb, os.WriteFile(filepath.Join(backupPath, "LATEST"), []byte("002"), perm.PublicFile))
root := gittest.WriteCommit(tb, cfg, expectedRepoPath,
gittest.WithBranch("master"),
@@ -613,25 +616,35 @@ func TestManager_Restore_latest(t *testing.T) {
gittest.WithParents(root),
)
gittest.Exec(tb, cfg, "-C", expectedRepoPath, "symbolic-ref", "HEAD", "refs/heads/master")
- bundlePath1 := filepath.Join(backupPath, "001.bundle")
- gittest.Exec(tb, cfg, "-C", expectedRepoPath, "bundle", "create", bundlePath1,
+ bundle1 := gittest.Exec(tb, cfg, "-C", expectedRepoPath, "bundle", "create", "-",
"HEAD",
"refs/heads/master",
"refs/heads/other",
)
+ refs1 := gittest.Exec(t, cfg, "-C", expectedRepoPath, "show-ref", "--head")
master2 := gittest.WriteCommit(tb, cfg, expectedRepoPath,
gittest.WithBranch("master"),
gittest.WithParents(master1),
)
- bundlePath2 := filepath.Join(backupPath, "002.bundle")
- gittest.Exec(tb, cfg, "-C", expectedRepoPath, "bundle", "create", bundlePath2,
+ bundle2 := gittest.Exec(tb, cfg, "-C", expectedRepoPath, "bundle", "create", "-",
"HEAD",
"^"+master1.String(),
"^"+other.String(),
"refs/heads/master",
"refs/heads/other",
)
+ refs2 := gittest.Exec(t, cfg, "-C", expectedRepoPath, "show-ref", "--head")
+
+ relativePath := stripRelativePath(tb, repo)
+ testhelper.WriteFiles(tb, backupRoot, map[string]any{
+ filepath.Join(relativePath, "LATEST"): backupID,
+ filepath.Join(relativePath, backupID, "LATEST"): "002",
+ filepath.Join(relativePath, backupID, "001.bundle"): bundle1,
+ filepath.Join(relativePath, backupID, "002.bundle"): bundle2,
+ filepath.Join(relativePath, backupID, "001.refs"): refs1,
+ filepath.Join(relativePath, backupID, "002.refs"): refs2,
+ })
checksum := new(git.Checksum)
checksum.Add(git.NewReference("HEAD", master2))
@@ -761,6 +774,8 @@ func TestManager_Restore_specific(t *testing.T) {
commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main"))
gittest.WriteTag(t, cfg, repoPath, "v1.0.0", commitID.Revision())
repoChecksum := gittest.ChecksumRepo(t, cfg, repoPath)
+ repoBundle := gittest.BundleRepo(t, cfg, repoPath, "-")
+ repoRefs := gittest.Exec(t, cfg, "-C", repoPath, "show-ref", "--head")
backupRoot := testhelper.TempDir(t)
@@ -776,12 +791,14 @@ func TestManager_Restore_specific(t *testing.T) {
desc: "single incremental",
setup: func(tb testing.TB) (*gitalypb.Repository, *git.Checksum) {
repo, _ := gittest.CreateRepository(t, ctx, cfg)
- repoBackupPath := joinBackupPath(tb, backupRoot, repo)
- backupPath := filepath.Join(repoBackupPath, backupID)
- require.NoError(tb, os.MkdirAll(backupPath, perm.PublicDir))
- require.NoError(tb, os.WriteFile(filepath.Join(backupPath, "LATEST"), []byte("001"), perm.PublicFile))
- bundlePath := filepath.Join(backupPath, "001.bundle")
- gittest.BundleRepo(tb, cfg, repoPath, bundlePath)
+
+ relativePath := stripRelativePath(tb, repo)
+ testhelper.WriteFiles(tb, backupRoot, map[string]any{
+ filepath.Join(relativePath, "LATEST"): backupID,
+ filepath.Join(relativePath, backupID, "LATEST"): "001",
+ filepath.Join(relativePath, backupID, "001.bundle"): repoBundle,
+ filepath.Join(relativePath, backupID, "001.refs"): repoRefs,
+ })
return repo, repoChecksum
},
@@ -793,10 +810,6 @@ func TestManager_Restore_specific(t *testing.T) {
_, expectedRepoPath := gittest.CreateRepository(t, ctx, cfg)
repo, _ := gittest.CreateRepository(t, ctx, cfg)
- repoBackupPath := joinBackupPath(tb, backupRoot, repo)
- backupPath := filepath.Join(repoBackupPath, backupID)
- require.NoError(tb, os.MkdirAll(backupPath, perm.PublicDir))
- require.NoError(tb, os.WriteFile(filepath.Join(backupPath, "LATEST"), []byte("002"), perm.PublicFile))
root := gittest.WriteCommit(tb, cfg, expectedRepoPath,
gittest.WithBranch("master"),
@@ -810,25 +823,35 @@ func TestManager_Restore_specific(t *testing.T) {
gittest.WithParents(root),
)
gittest.Exec(tb, cfg, "-C", expectedRepoPath, "symbolic-ref", "HEAD", "refs/heads/master")
- bundlePath1 := filepath.Join(backupPath, "001.bundle")
- gittest.Exec(tb, cfg, "-C", expectedRepoPath, "bundle", "create", bundlePath1,
+ bundle1 := gittest.Exec(tb, cfg, "-C", expectedRepoPath, "bundle", "create", "-",
"HEAD",
"refs/heads/master",
"refs/heads/other",
)
+ refs1 := gittest.Exec(t, cfg, "-C", expectedRepoPath, "show-ref", "--head")
master2 := gittest.WriteCommit(tb, cfg, expectedRepoPath,
gittest.WithBranch("master"),
gittest.WithParents(master1),
)
- bundlePath2 := filepath.Join(backupPath, "002.bundle")
- gittest.Exec(tb, cfg, "-C", expectedRepoPath, "bundle", "create", bundlePath2,
+ bundle2 := gittest.Exec(tb, cfg, "-C", expectedRepoPath, "bundle", "create", "-",
"HEAD",
"^"+master1.String(),
"^"+other.String(),
"refs/heads/master",
"refs/heads/other",
)
+ refs2 := gittest.Exec(t, cfg, "-C", expectedRepoPath, "show-ref", "--head")
+
+ relativePath := stripRelativePath(tb, repo)
+ testhelper.WriteFiles(tb, backupRoot, map[string]any{
+ filepath.Join(relativePath, "LATEST"): backupID,
+ filepath.Join(relativePath, backupID, "LATEST"): "002",
+ filepath.Join(relativePath, backupID, "001.bundle"): bundle1,
+ filepath.Join(relativePath, backupID, "002.bundle"): bundle2,
+ filepath.Join(relativePath, backupID, "001.refs"): refs1,
+ filepath.Join(relativePath, backupID, "002.refs"): refs2,
+ })
checksum := new(git.Checksum)
checksum.Add(git.NewReference("HEAD", master2))
diff --git a/internal/backup/locator.go b/internal/backup/locator.go
index 6d1419489..f2897f2e0 100644
--- a/internal/backup/locator.go
+++ b/internal/backup/locator.go
@@ -61,10 +61,9 @@ func (l LegacyLocator) newFull(repo *gitalypb.Repository) *Step {
backupPath := strings.TrimSuffix(repo.RelativePath, ".git")
return &Step{
- SkippableOnNotFound: true,
- BundlePath: backupPath + ".bundle",
- RefPath: backupPath + ".refs",
- CustomHooksPath: filepath.Join(backupPath, "custom_hooks.tar"),
+ BundlePath: backupPath + ".bundle",
+ RefPath: backupPath + ".refs",
+ CustomHooksPath: filepath.Join(backupPath, "custom_hooks.tar"),
}
}
diff --git a/internal/backup/locator_test.go b/internal/backup/locator_test.go
index bb30379fd..01534d368 100644
--- a/internal/backup/locator_test.go
+++ b/internal/backup/locator_test.go
@@ -35,10 +35,9 @@ func TestLegacyLocator(t *testing.T) {
t.Parallel()
expected := &Step{
- SkippableOnNotFound: true,
- BundlePath: repo.RelativePath + ".bundle",
- RefPath: repo.RelativePath + ".refs",
- CustomHooksPath: filepath.Join(repo.RelativePath, "custom_hooks.tar"),
+ BundlePath: repo.RelativePath + ".bundle",
+ RefPath: repo.RelativePath + ".refs",
+ CustomHooksPath: filepath.Join(repo.RelativePath, "custom_hooks.tar"),
}
full := l.BeginFull(ctx, repo, "abc123")
@@ -54,10 +53,9 @@ func TestLegacyLocator(t *testing.T) {
ObjectFormat: git.ObjectHashSHA1.Format,
Steps: []Step{
{
- SkippableOnNotFound: true,
- BundlePath: repo.RelativePath + ".bundle",
- RefPath: repo.RelativePath + ".refs",
- CustomHooksPath: filepath.Join(repo.RelativePath, "custom_hooks.tar"),
+ BundlePath: repo.RelativePath + ".bundle",
+ RefPath: repo.RelativePath + ".refs",
+ CustomHooksPath: filepath.Join(repo.RelativePath, "custom_hooks.tar"),
},
},
}
@@ -238,10 +236,9 @@ func TestPointerLocator(t *testing.T) {
ObjectFormat: git.ObjectHashSHA1.Format,
Steps: []Step{
{
- SkippableOnNotFound: true,
- BundlePath: repo.RelativePath + ".bundle",
- RefPath: repo.RelativePath + ".refs",
- CustomHooksPath: filepath.Join(repo.RelativePath, "custom_hooks.tar"),
+ BundlePath: repo.RelativePath + ".bundle",
+ RefPath: repo.RelativePath + ".refs",
+ CustomHooksPath: filepath.Join(repo.RelativePath, "custom_hooks.tar"),
},
},
}
diff --git a/internal/backup/server_side_test.go b/internal/backup/server_side_test.go
index fc5e990ac..3329c8a78 100644
--- a/internal/backup/server_side_test.go
+++ b/internal/backup/server_side_test.go
@@ -1,3 +1,5 @@
+//go:build !gitaly_test_sha256
+
package backup_test
import (
@@ -192,7 +194,7 @@ func TestServerSideAdapter_Restore(t *testing.T) {
backupID: "",
}
},
- expectedErr: fmt.Errorf("server-side restore: %w: rpc error: code = FailedPrecondition desc = restore repository: manager: repository skipped: restore bundle: \"@test/restore/latest/missing.bundle\": doesn't exist", backup.ErrSkipped),
+ expectedErr: fmt.Errorf("server-side restore: %w: rpc error: code = FailedPrecondition desc = restore repository: manager: repository skipped: read refs: doesn't exist", backup.ErrSkipped),
},
} {
tc := tc
A merge request may have already been submitted, check that on [merge commit commit](c1ef53276082, as usually changes in Gitaly are not breaking, compliance becoming mandatory in the following development cycles.