shithub: furgit

Download patch

ref: 4e43d7207bf91ee759c770de1bcc8562a71b4aca
parent: d314d1f7e933ca83081eec289aa0cb6e75a7eeac
author: Runxi Yu <runxiyu@umich.edu>
date: Sun Mar 22 14:00:22 EDT 2026

objectstore/*, repository, receivepack/service: don't take ownership of root

--- a/objectstore/loose/store.go
+++ b/objectstore/loose/store.go
@@ -12,12 +12,10 @@
 // Loose objects are zlib streams whose trailer uses Adler-32. Which reads
 // consume enough of the stream to reach and verify that trailer is documented
 // on the individual methods.
-//
-// Store owns root and closes it in Close.
 type Store struct {
 	// root is the objects directory capability used for all object file access.
 	// Object files are opened by relative paths like "<first2>/<rest>".
-	// Store owns this root.
+	// Store borrows this root.
 	root *os.Root
 	// algo is the expected object ID algorithm for lookups.
 	algo objectid.Algorithm
@@ -37,7 +35,7 @@
 
 // Close releases resources associated with the backend.
 //
+// Store borrows its root, so Close does not close it.
+//
 // Repeated calls to Close are undefined behavior.
-func (store *Store) Close() error {
-	return store.root.Close()
-}
+func (store *Store) Close() error { return nil }
--- a/objectstore/packed/close.go
+++ b/objectstore/packed/close.go
@@ -2,10 +2,11 @@
 
 // Close releases mapped pack/index resources associated with the store.
 //
+// Store borrows its root, so Close does not close it.
+//
 // Repeated calls to Close are undefined behavior.
 func (store *Store) Close() error {
 	store.stateMu.Lock()
-	root := store.root
 	packs := store.packs
 	store.stateMu.Unlock()
 	store.idxMu.RLock()
@@ -31,11 +32,6 @@
 	store.cacheMu.Lock()
 	store.deltaCache.clear()
 	store.cacheMu.Unlock()
-
-	err := root.Close()
-	if err != nil && closeErr == nil {
-		closeErr = err
-	}
 
 	return closeErr
 }
--- a/objectstore/packed/store.go
+++ b/objectstore/packed/store.go
@@ -11,10 +11,8 @@
 )
 
 // Store reads Git objects from pack/index files under an objects/pack root.
-//
-// Store owns root and closes it in Close.
 type Store struct {
-	// root is the objects/pack capability used for all file access.
+	// root is the borrowed objects/pack capability used for all file access.
 	root *os.Root
 	// algo is the expected object ID algorithm for lookups.
 	algo objectid.Algorithm
--- a/receivepack/service/quarantine.go
+++ b/receivepack/service/quarantine.go
@@ -14,6 +14,11 @@
 
 // createQuarantineRoot creates one per-push quarantine directory beneath the
 // permanent objects root.
+//
+// It returns both the quarantine directory name relative to ObjectsRoot and an
+// opened root for that directory. Callers use the name for later promotion or
+// removal relative to ObjectsRoot, and use the opened root for capability-based
+// access within the quarantine itself.
 func (service *Service) createQuarantineRoot() (string, *os.Root, error) {
 	name := "tmp_objdir-incoming-" + rand.Text()
 
--- a/receivepack/service/quarantine_objects.go
+++ /dev/null
@@ -1,50 +1,0 @@
-package service
-
-import (
-	"os"
-
-	"codeberg.org/lindenii/furgit/objectstore"
-	"codeberg.org/lindenii/furgit/objectstore/loose"
-	"codeberg.org/lindenii/furgit/objectstore/memory"
-	objectmix "codeberg.org/lindenii/furgit/objectstore/mix"
-	"codeberg.org/lindenii/furgit/objectstore/packed"
-)
-
-func (service *Service) openQuarantinedObjects(quarantineName string) (objectstore.Store, error) {
-	if quarantineName == "" {
-		return memory.New(service.opts.Algorithm), nil
-	}
-
-	looseRoot, err := service.opts.ObjectsRoot.OpenRoot(quarantineName)
-	if err != nil {
-		return nil, err
-	}
-
-	looseStore, err := loose.New(looseRoot, service.opts.Algorithm)
-	if err != nil {
-		_ = looseRoot.Close()
-
-		return nil, err
-	}
-
-	packRoot, err := looseRoot.OpenRoot("pack")
-	if err == nil {
-		packedStore, packedErr := packed.New(packRoot, service.opts.Algorithm, packed.Options{})
-		if packedErr != nil {
-			_ = packRoot.Close()
-			_ = looseStore.Close()
-
-			return nil, packedErr
-		}
-
-		return objectmix.New(looseStore, packedStore), nil
-	}
-
-	if !os.IsNotExist(err) {
-		_ = looseStore.Close()
-
-		return nil, err
-	}
-
-	return looseStore, nil
-}
--- a/receivepack/service/run_hook.go
+++ b/receivepack/service/run_hook.go
@@ -2,8 +2,13 @@
 
 import (
 	"context"
+	"os"
 
 	"codeberg.org/lindenii/furgit/internal/utils"
+	"codeberg.org/lindenii/furgit/objectstore"
+	"codeberg.org/lindenii/furgit/objectstore/loose"
+	objectmix "codeberg.org/lindenii/furgit/objectstore/mix"
+	"codeberg.org/lindenii/furgit/objectstore/packed"
 )
 
 func (service *Service) runHook(
@@ -32,16 +37,69 @@
 
 	utils.BestEffortFprintf(service.opts.Progress, "running hooks...\r")
 
-	quarantinedObjects, err := service.openQuarantinedObjects(quarantineName)
-	if err != nil {
-		utils.BestEffortFprintf(service.opts.Progress, "running hooks: failed: %v.\n", err)
+	quarantinedObjects := service.opts.ExistingObjects
+	var (
+		quarantineObjectsStore objectstore.Store
+		quarantineLooseRoot    *os.Root
+		quarantinePackRoot     *os.Root
+		err                    error
+	)
 
-		return nil, nil, nil, false, err.Error()
-	}
+	if quarantineName != "" {
+		quarantineLooseRoot, err = service.opts.ObjectsRoot.OpenRoot(quarantineName)
+		if err != nil {
+			utils.BestEffortFprintf(service.opts.Progress, "running hooks: failed: %v.\n", err)
 
-	defer func() {
-		_ = quarantinedObjects.Close()
-	}()
+			return nil, nil, nil, false, err.Error()
+		}
+
+		quarantineLooseStore, err := loose.New(quarantineLooseRoot, service.opts.Algorithm)
+		if err != nil {
+			_ = quarantineLooseRoot.Close()
+			utils.BestEffortFprintf(service.opts.Progress, "running hooks: failed: %v.\n", err)
+
+			return nil, nil, nil, false, err.Error()
+		}
+
+		quarantineObjectsStore = quarantineLooseStore
+		quarantinedObjects = quarantineLooseStore
+
+		quarantinePackRoot, err = quarantineLooseRoot.OpenRoot("pack")
+		if err == nil {
+			quarantinePackedStore, packedErr := packed.New(quarantinePackRoot, service.opts.Algorithm, packed.Options{})
+			if packedErr != nil {
+				_ = quarantineLooseStore.Close()
+				_ = quarantinePackRoot.Close()
+				_ = quarantineLooseRoot.Close()
+				utils.BestEffortFprintf(service.opts.Progress, "running hooks: failed: %v.\n", packedErr)
+
+				return nil, nil, nil, false, packedErr.Error()
+			}
+
+			quarantineObjectsStore = objectmix.New(quarantineLooseStore, quarantinePackedStore)
+			quarantinedObjects = quarantineObjectsStore
+		} else if !os.IsNotExist(err) {
+			_ = quarantineLooseStore.Close()
+			_ = quarantineLooseRoot.Close()
+			utils.BestEffortFprintf(service.opts.Progress, "running hooks: failed: %v.\n", err)
+
+			return nil, nil, nil, false, err.Error()
+		}
+
+		defer func() {
+			if quarantineObjectsStore != nil {
+				_ = quarantineObjectsStore.Close()
+			}
+
+			if quarantinePackRoot != nil {
+				_ = quarantinePackRoot.Close()
+			}
+
+			if quarantineLooseRoot != nil {
+				_ = quarantineLooseRoot.Close()
+			}
+		}()
+	}
 
 	decisions, err := service.opts.Hook(ctx, HookRequest{
 		Refs:               service.opts.Refs,
--- a/repository/close.go
+++ b/repository/close.go
@@ -28,5 +28,26 @@
 		}
 	}
 
+	if repo.objectsWriteRoot != nil {
+		err := repo.objectsWriteRoot.Close()
+		if err != nil {
+			errs = append(errs, err)
+		}
+	}
+
+	if repo.objectsPackRoot != nil {
+		err := repo.objectsPackRoot.Close()
+		if err != nil {
+			errs = append(errs, err)
+		}
+	}
+
+	if repo.objectsRoot != nil {
+		err := repo.objectsRoot.Close()
+		if err != nil {
+			errs = append(errs, err)
+		}
+	}
+
 	return errors.Join(errs...)
 }
--- a/repository/objects.go
+++ b/repository/objects.go
@@ -13,59 +13,82 @@
 )
 
 //nolint:ireturn
-func openObjectStore(root *os.Root, algo objectid.Algorithm) (objectstore.Store, *objectloose.Store, error) {
-	objectsRoot, err := root.OpenRoot("objects")
+func openObjectStore(
+	root *os.Root,
+	algo objectid.Algorithm,
+) (
+	objects objectstore.Store,
+	objectsRoot *os.Root,
+	objectsPackRoot *os.Root,
+	objectsLooseForWritingOnly *objectloose.Store,
+	objectsWriteRoot *os.Root,
+	err error,
+) {
+	objectsRoot, err = root.OpenRoot("objects")
 	if err != nil {
-		return nil, nil, fmt.Errorf("repository: open objects: %w", err)
+		return nil, nil, nil, nil, nil, fmt.Errorf("repository: open objects: %w", err)
 	}
 
 	looseStore, err := objectloose.New(objectsRoot, algo)
 	if err != nil {
-		return nil, nil, err
+		_ = objectsRoot.Close()
+
+		return nil, nil, nil, nil, nil, err
 	}
 
 	backends := []objectstore.Store{looseStore}
+	objectsPackRoot, err = objectsRoot.OpenRoot("pack")
 
-	packRoot, err := objectsRoot.OpenRoot("pack")
 	if err == nil {
 		var packedStore *objectpacked.Store
 
 		packedStore, err = objectpacked.New(
-			packRoot,
+			objectsPackRoot,
 			algo,
 			objectpacked.Options{RefreshPolicy: objectpacked.RefreshPolicyNever},
 		)
 		if err != nil {
+			_ = objectsPackRoot.Close()
 			_ = looseStore.Close()
+			_ = objectsRoot.Close()
 
-			return nil, nil, err
+			return nil, nil, nil, nil, nil, err
 		}
 
 		backends = append(backends, packedStore)
 	} else if !errors.Is(err, os.ErrNotExist) {
 		_ = looseStore.Close()
+		_ = objectsRoot.Close()
 
-		return nil, nil, fmt.Errorf("repository: open objects/pack: %w", err)
+		return nil, nil, nil, nil, nil, fmt.Errorf("repository: open objects/pack: %w", err)
 	}
 
-	objectsChain := objectmix.New(backends...)
+	objects = objectmix.New(backends...)
 
-	objectsRootForWriting, err := root.OpenRoot("objects")
+	objectsWriteRoot, err = root.OpenRoot("objects")
 	if err != nil {
-		_ = objectsChain.Close()
+		_ = objects.Close()
+		if objectsPackRoot != nil {
+			_ = objectsPackRoot.Close()
+		}
+		_ = objectsRoot.Close()
 
-		return nil, nil, fmt.Errorf("repository: open objects for loose writing: %w", err)
+		return nil, nil, nil, nil, nil, fmt.Errorf("repository: open objects for loose writing: %w", err)
 	}
 
-	objectsLooseForWritingOnly, err := objectloose.New(objectsRootForWriting, algo)
+	objectsLooseForWritingOnly, err = objectloose.New(objectsWriteRoot, algo)
 	if err != nil {
-		_ = objectsRootForWriting.Close()
-		_ = objectsChain.Close()
+		_ = objects.Close()
+		_ = objectsWriteRoot.Close()
+		if objectsPackRoot != nil {
+			_ = objectsPackRoot.Close()
+		}
+		_ = objectsRoot.Close()
 
-		return nil, nil, err
+		return nil, nil, nil, nil, nil, err
 	}
 
-	return objectsChain, objectsLooseForWritingOnly, nil
+	return objects, objectsRoot, objectsPackRoot, objectsLooseForWritingOnly, objectsWriteRoot, nil
 }
 
 // Objects returns the configured object store.
--- a/repository/open.go
+++ b/repository/open.go
@@ -28,13 +28,16 @@
 
 	repo.algo = algo
 
-	objects, objectsLooseForWritingOnly, err := openObjectStore(root, algo)
+	objects, objectsRoot, objectsPackRoot, objectsLooseForWritingOnly, objectsWriteRoot, err := openObjectStore(root, algo)
 	if err != nil {
 		return nil, err
 	}
 
 	repo.objects = objects
+	repo.objectsRoot = objectsRoot
+	repo.objectsPackRoot = objectsPackRoot
 	repo.objectsLooseForWritingOnly = objectsLooseForWritingOnly
+	repo.objectsWriteRoot = objectsWriteRoot
 
 	refs, err := openRefStore(root, algo, detectPackedRefsTimeout(cfg))
 	if err != nil {
--- a/repository/repository.go
+++ b/repository/repository.go
@@ -2,6 +2,8 @@
 package repository
 
 import (
+	"os"
+
 	"codeberg.org/lindenii/furgit/config"
 	"codeberg.org/lindenii/furgit/objectid"
 	"codeberg.org/lindenii/furgit/objectstore"
@@ -18,6 +20,9 @@
 	algo   objectid.Algorithm
 
 	objects                    objectstore.Store
+	objectsRoot                *os.Root
+	objectsPackRoot            *os.Root
 	objectsLooseForWritingOnly *objectloose.Store
+	objectsWriteRoot           *os.Root
 	refs                       refstore.ReadWriteStore
 }
--