Skip to content
Snippets Groups Projects
Commit 4d1e0d13 authored by Justin Tobler's avatar Justin Tobler
Browse files

objectpool: Refactor reading alternates file

Both `objectpool.FromRepo()` and `objectpool.LinkedToRepository()` use
`getAlternateObjectDir()` to read the alternates file. Refactor the code
to use `stats.AlternatesInfoForRepository()` instead because it is more
robust and simplifies error handling.

The only behavior change is that `objectpool.FromRepo()` is slightly
more strict and returns an error if a repository contains an empty
alternates file. Previously it would only return an error if the
alternates file was missing or malformed. An empty alternates file
should be treated as invalid and return an error in the same manner.
parent e852ee81
No related branches found
No related tags found
Loading
......@@ -2,7 +2,6 @@ package objectpool
import (
"context"
"errors"
"fmt"
"io"
"os"
......@@ -10,6 +9,7 @@ import (
"strings"
"gitlab.com/gitlab-org/gitaly/v16/internal/git/localrepo"
"gitlab.com/gitlab-org/gitaly/v16/internal/git/stats"
"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage/storagectx"
"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/transaction"
"gitlab.com/gitlab-org/gitaly/v16/internal/safe"
......@@ -159,14 +159,21 @@ func (o *ObjectPool) LinkedToRepository(repo *localrepo.Repo) (bool, error) {
return false, fmt.Errorf("getting object pool path: %w", err)
}
relPath, err := getAlternateObjectDir(repo)
repoPath, err := repo.Path()
if err != nil {
if errors.Is(err, ErrAlternateObjectDirNotExist) {
return false, nil
}
return false, err
return false, fmt.Errorf("getting repo path: %w", err)
}
altInfo, err := stats.AlternatesInfoForRepository(repoPath)
if err != nil {
return false, fmt.Errorf("getting alternates info: %w", err)
}
if !altInfo.Exists || len(altInfo.ObjectDirectories) == 0 {
return false, nil
}
relPath := altInfo.ObjectDirectories[0]
expectedRelPath, err := o.getRelativeObjectPath(repo)
if err != nil {
return false, err
......
package objectpool
import (
"bufio"
"bytes"
"context"
"errors"
"fmt"
"io"
"os"
"path/filepath"
"strings"
......@@ -15,6 +12,7 @@ import (
"gitlab.com/gitlab-org/gitaly/v16/internal/git/catfile"
housekeepingmgr "gitlab.com/gitlab-org/gitaly/v16/internal/git/housekeeping/manager"
"gitlab.com/gitlab-org/gitaly/v16/internal/git/localrepo"
"gitlab.com/gitlab-org/gitaly/v16/internal/git/stats"
"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/transaction"
"gitlab.com/gitlab-org/gitaly/v16/internal/log"
......@@ -163,15 +161,16 @@ func FromRepo(
return nil, err
}
relativeAlternateObjectDirPath, err := getAlternateObjectDir(repo)
altInfo, err := stats.AlternatesInfoForRepository(repoPath)
if err != nil {
return nil, err
return nil, fmt.Errorf("getting alternates info: %w", err)
}
if relativeAlternateObjectDirPath == "" {
return nil, nil
if !altInfo.Exists || len(altInfo.ObjectDirectories) == 0 {
return nil, ErrAlternateObjectDirNotExist
}
absolutePoolObjectDirPath := filepath.Join(repoPath, "objects", relativeAlternateObjectDirPath)
absolutePoolObjectDirPath := altInfo.AbsoluteObjectDirectories()[0]
relativePoolObjectDirPath, err := filepath.Rel(storagePath, absolutePoolObjectDirPath)
if err != nil {
return nil, err
......@@ -190,41 +189,3 @@ func FromRepo(
return FromProto(logger, locator, gitCmdFactory, catfileCache, txManager, housekeepingManager, objectPoolProto)
}
// getAlternateObjectDir returns the entry in the objects/info/attributes file if it exists
// it will only return the first line of the file if there are multiple lines.
func getAlternateObjectDir(repo *localrepo.Repo) (string, error) {
altPath, err := repo.InfoAlternatesPath()
if err != nil {
return "", err
}
if _, err = os.Stat(altPath); err != nil {
if os.IsNotExist(err) {
return "", ErrAlternateObjectDirNotExist
}
return "", err
}
altFile, err := os.Open(altPath)
if err != nil {
return "", err
}
defer altFile.Close()
r := bufio.NewReader(altFile)
b, err := r.ReadBytes('\n')
if err != nil && !errors.Is(err, io.EOF) {
return "", fmt.Errorf("reading alternates file: %w", err)
}
if err == nil {
b = b[:len(b)-1]
}
if bytes.HasPrefix(b, []byte("#")) {
return "", ErrAlternateObjectDirNotExist
}
return string(b), nil
}
......@@ -97,7 +97,7 @@ func TestFromRepo_failures(t *testing.T) {
{
desc: "alternates is empty",
fileContent: nil,
expectedErr: nil,
expectedErr: ErrAlternateObjectDirNotExist,
},
{
desc: "alternates is commented",
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment