Read blob sizes only for selected validation inputs
This commit is contained in:
@@ -59,22 +59,24 @@ workspace-file check using the same candidate inventory. Resolved module bodies
|
||||
are parsed once per blob object ID; only owned filesystem-replacement targets
|
||||
and line numbers remain cached. Each declaration path receives findings from
|
||||
that summary in its own path context.
|
||||
The optional file-size check uses each blob's captured Git size, including
|
||||
symlink link-text bytes, and skips entries without a blob size such as gitlinks.
|
||||
It applies normalized literal file/directory exclusions with path-boundary
|
||||
matching, then returns the first oversized path with `FileSizeMeasurement`.
|
||||
The optional file-size check applies normalized literal file/directory
|
||||
exclusions with path-boundary matching, then requests local Git sizes for the
|
||||
remaining selected blobs. Sizes include symlink link-text bytes; gitlinks are
|
||||
omitted. The check returns the first oversized path with `FileSizeMeasurement`.
|
||||
The optional Markdown check follows file-size inspection and uses the same
|
||||
candidate tree. It parses regular Markdown blobs, resolves local destinations,
|
||||
and checks supported anchors; outside-checkout and submodule observations are
|
||||
written to the shared output writer. A nil writer keeps library use quiet.
|
||||
`CandidateTree` gets metadata and bytes from Git objects, reads blob bytes
|
||||
lazily, and resolves destinations without consulting the worktree or
|
||||
neighboring files. Each tree owns one lazy Git blob reader; direct tree users
|
||||
must close it when finished. The shared validation executor closes its tree on
|
||||
every return before product checks or build callbacks run. A metadata-only
|
||||
selection never starts the reader. Per-read contexts cancel an in-flight blob
|
||||
exchange. The context supplied when creating the tree governs the reader
|
||||
lifetime, including shutdown and child reaping.
|
||||
`CandidateTree` enumerates full repository path, type, mode, and object metadata
|
||||
without requiring blob contents or sizes. Checks acquire sizes and bytes for
|
||||
their selected inputs and required references. Unreferenced blobs in excluded
|
||||
modules remain outside those reads. Destinations resolve without consulting
|
||||
the worktree or neighboring files. Each tree owns one lazy Git blob reader;
|
||||
direct tree users must close it when finished. The shared validation executor
|
||||
closes its tree on every return before product checks or build callbacks run.
|
||||
A metadata-only selection never starts the reader. Per-read contexts cancel
|
||||
an in-flight blob exchange. The context supplied when creating the tree governs
|
||||
the reader lifetime, including shutdown and child reaping.
|
||||
Regular-file reads return independent caller-owned byte slices and are not
|
||||
retained by the tree. Symlink targets are retained as immutable owned strings
|
||||
by blob object ID; explicitly reading a symlink still returns independent
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
# Remediation: module release implementation review
|
||||
|
||||
Status: open; issues 1–14 are complete, and issue 15 requires remediation.
|
||||
Status: complete; all fifteen findings addressed and verified.
|
||||
The initial review examined `90559fe`, compared with planning baseline `dbfae9e`.
|
||||
The follow-up repository audit examined `6b0b8ce`, including those corrections.
|
||||
|
||||
@@ -544,7 +544,18 @@ network-capable preparation must continue to work.
|
||||
|
||||
## Issue 15: Limit blob availability requirements to selected validation inputs (P2)
|
||||
|
||||
Status: open. Category: module-scope leakage through eager metadata enrichment.
|
||||
Status: complete. Category: module-scope leakage through eager metadata enrichment.
|
||||
|
||||
**Completion:** Tree enumeration now acquires path/type/mode/object metadata
|
||||
independently of blobs. The size check requests sizes only for selected,
|
||||
non-excluded blobs; other checks read contents and references when needed.
|
||||
Filtered sparse-clone regressions cover formatting, sizes, workspace metadata,
|
||||
explicit shared selection, exclusions, required shared contents, and required
|
||||
Markdown references. They failed before the correction and now preserve each
|
||||
check's scope and error identity without transport or object-database changes.
|
||||
Existing size, symlink, gitlink, vendor, boundary, reader/cache, and cancellation
|
||||
tests pass. The complete offline suite, compiled API/dependency check,
|
||||
documentation links, and whitespace checks pass for all four follow-up fixes.
|
||||
|
||||
**Evidence:** [ReadCandidateTreeForScope](../../internal/validation/tree.go)
|
||||
calls [ReadTree](../../internal/git/source.go) before constructing module and
|
||||
|
||||
@@ -163,8 +163,9 @@ func CommittedPaths(repositoryDir, commit string) PathResolver {
|
||||
}
|
||||
}
|
||||
|
||||
// ReadTree enumerates all entries in a commit and records blob sizes. Paths
|
||||
// are NUL-delimited by Git so whitespace in a filename is preserved.
|
||||
// ReadTree enumerates path, type, mode, and object metadata for a commit.
|
||||
// Blob sizes and contents are inspected separately. Paths are NUL-delimited
|
||||
// by Git so whitespace in a filename is preserved.
|
||||
func ReadTree(ctx context.Context, repositoryDir, commit string) ([]TreeEntry, error) {
|
||||
if !validObjectID(commit) {
|
||||
return nil, fmt.Errorf("invalid commit object ID %q", commit)
|
||||
@@ -177,9 +178,6 @@ func ReadTree(ctx context.Context, repositoryDir, commit string) ([]TreeEntry, e
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("parse committed tree %s: %w", commit, err)
|
||||
}
|
||||
if err := populateBlobSizes(ctx, repositoryDir, entries); err != nil {
|
||||
return nil, fmt.Errorf("read committed blob sizes for %s: %w", commit, err)
|
||||
}
|
||||
return entries, nil
|
||||
}
|
||||
|
||||
@@ -223,11 +221,16 @@ func parseTreeEntries(output []byte) ([]TreeEntry, error) {
|
||||
return entries, nil
|
||||
}
|
||||
|
||||
func populateBlobSizes(ctx context.Context, repositoryDir string, entries []TreeEntry) error {
|
||||
// PopulateBlobSizes fills unknown sizes for the supplied blob entries using
|
||||
// local objects. Callers select the entries whose sizes their check needs.
|
||||
func PopulateBlobSizes(ctx context.Context, repositoryDir string, entries []TreeEntry) error {
|
||||
if err := ctx.Err(); err != nil {
|
||||
return err
|
||||
}
|
||||
var blobIndexes []int
|
||||
var input strings.Builder
|
||||
for index := range entries {
|
||||
if entries[index].Type != "blob" {
|
||||
if entries[index].Type != "blob" || entries[index].HasSize {
|
||||
continue
|
||||
}
|
||||
blobIndexes = append(blobIndexes, index)
|
||||
@@ -250,7 +253,7 @@ func populateBlobSizes(ctx context.Context, repositoryDir string, entries []Tree
|
||||
fields := strings.Fields(line)
|
||||
entry := &entries[blobIndexes[index]]
|
||||
if len(fields) == 2 && fields[0] == entry.ObjectID && fields[1] == "missing" {
|
||||
return fmt.Errorf("committed blob %s is unavailable locally", entry.ObjectID)
|
||||
return fmt.Errorf("committed blob %s for %q is unavailable locally", entry.ObjectID, entry.Path)
|
||||
}
|
||||
if len(fields) != 3 || fields[0] != entry.ObjectID || fields[1] != "blob" {
|
||||
return fmt.Errorf("unexpected blob size record %q", line)
|
||||
|
||||
@@ -85,8 +85,18 @@ func TestCandidateObjectReadsStayLocalInPartialClones(t *testing.T) {
|
||||
t.Setenv("RELEASEKIT_GIT_ATTEMPTS", attempts)
|
||||
t.Setenv("GIT_NO_LAZY_FETCH", "0")
|
||||
t.Setenv("GIT_ALLOW_PROTOCOL", "ssh")
|
||||
if _, err := ReadTree(t.Context(), repository, fixture.commit); err == nil {
|
||||
t.Fatal("tree inspection succeeded with unavailable objects")
|
||||
entries, treeErr := ReadTree(t.Context(), repository, fixture.commit)
|
||||
if filter == "tree:0" {
|
||||
if treeErr == nil {
|
||||
t.Fatal("tree inspection succeeded with unavailable trees")
|
||||
}
|
||||
} else {
|
||||
if treeErr != nil {
|
||||
t.Fatalf("metadata inspection required unavailable blobs: %v", treeErr)
|
||||
}
|
||||
if err := PopulateBlobSizes(t.Context(), repository, entries); err == nil {
|
||||
t.Fatal("size inspection succeeded with unavailable blobs")
|
||||
}
|
||||
}
|
||||
_, fileErr := ReadCommitFile(t.Context(), repository, fixture.commit, "source.txt")
|
||||
var fileExitError *exec.ExitError
|
||||
|
||||
@@ -1,21 +1,31 @@
|
||||
package validation
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"path"
|
||||
"strings"
|
||||
|
||||
"gitea.maximumdirect.net/eric/ReleaseKit/internal/git"
|
||||
)
|
||||
|
||||
const defaultFileSizeLimit int64 = 1_000_000
|
||||
|
||||
func checkFileSize(tree *CandidateTree, settings *FileSizeSettings) error {
|
||||
func checkFileSize(ctx context.Context, tree *CandidateTree, settings *FileSizeSettings) error {
|
||||
entries := make([]git.TreeEntry, 0, len(tree.inputEntries))
|
||||
for _, entry := range tree.inputEntries {
|
||||
if entry.Type == "blob" && !excludedByPath(entry.Path, settings.Exclusions) {
|
||||
entries = append(entries, entry)
|
||||
}
|
||||
}
|
||||
if err := git.PopulateBlobSizes(ctx, tree.repositoryDir, entries); err != nil {
|
||||
return &Failure{Check: FileSize, Cause: fmt.Errorf("inspect selected committed file sizes: %w", err)}
|
||||
}
|
||||
limit := defaultFileSizeLimit
|
||||
if settings.LimitBytes != nil {
|
||||
limit = *settings.LimitBytes
|
||||
}
|
||||
for _, entry := range tree.inputEntries {
|
||||
if entry.Type != "blob" || !entry.HasSize || excludedByPath(entry.Path, settings.Exclusions) {
|
||||
continue
|
||||
}
|
||||
for _, entry := range entries {
|
||||
if entry.Size > limit {
|
||||
return &Failure{
|
||||
Check: FileSize,
|
||||
|
||||
@@ -33,7 +33,7 @@ func TestCheckFileSizeUsesStrictLimitsAndDefault(t *testing.T) {
|
||||
Path: "file.bin", Type: "blob", Mode: "100644", Size: test.size, HasSize: true,
|
||||
}}
|
||||
tree := &CandidateTree{entries: entries, inputEntries: entries}
|
||||
err := checkFileSize(tree, &FileSizeSettings{LimitBytes: test.limit})
|
||||
err := checkFileSize(t.Context(), tree, &FileSizeSettings{LimitBytes: test.limit})
|
||||
if !test.wantError {
|
||||
if err != nil {
|
||||
t.Fatalf("checkFileSize() error = %v, want nil", err)
|
||||
@@ -109,7 +109,7 @@ func TestCheckFileSizeAppliesLiteralPathExclusions(t *testing.T) {
|
||||
LimitBytes: int64Pointer(10),
|
||||
Exclusions: test.exclusions,
|
||||
}, ".")
|
||||
err := checkFileSize(&CandidateTree{entries: test.entries, inputEntries: test.entries}, settings)
|
||||
err := checkFileSize(t.Context(), &CandidateTree{entries: test.entries, inputEntries: test.entries}, settings)
|
||||
failure, ok := err.(*Failure)
|
||||
if !ok || failure.Check != FileSize || len(failure.Findings) != 1 || failure.Findings[0].Path != test.wantPath {
|
||||
t.Fatalf("checkFileSize() error = %v, want only %q to exceed the limit", err, test.wantPath)
|
||||
@@ -139,12 +139,12 @@ func TestCheckFileSizeUsesCommittedBlobSizes(t *testing.T) {
|
||||
}
|
||||
cleanupCandidateTree(t, tree)
|
||||
|
||||
err = checkFileSize(tree, &FileSizeSettings{LimitBytes: int64Pointer(3), Exclusions: []string{"link"}})
|
||||
err = checkFileSize(t.Context(), tree, &FileSizeSettings{LimitBytes: int64Pointer(3), Exclusions: []string{"link"}})
|
||||
if err != nil {
|
||||
t.Fatalf("checkFileSize() error = %v, want small binary and exact-limit blob to pass", err)
|
||||
}
|
||||
|
||||
err = checkFileSize(tree, &FileSizeSettings{LimitBytes: int64Pointer(3), Exclusions: []string{"small.bin", "boundary.bin"}})
|
||||
err = checkFileSize(t.Context(), tree, &FileSizeSettings{LimitBytes: int64Pointer(3), Exclusions: []string{"small.bin", "boundary.bin"}})
|
||||
failure, ok := err.(*Failure)
|
||||
if !ok || failure.Check != FileSize || len(failure.Findings) != 1 || failure.Findings[0].Path != "link" {
|
||||
t.Fatalf("checkFileSize() error = %v, want symlink blob to exceed the limit", err)
|
||||
@@ -153,7 +153,7 @@ func TestCheckFileSizeUsesCommittedBlobSizes(t *testing.T) {
|
||||
t.Fatalf("symlink measurement = %#v, want link-text size %d", failure.Measurement, len("small.bin"))
|
||||
}
|
||||
|
||||
err = checkFileSize(tree, &FileSizeSettings{LimitBytes: int64Pointer(0), Exclusions: []string{"small.bin", "boundary.bin", "link"}})
|
||||
err = checkFileSize(t.Context(), tree, &FileSizeSettings{LimitBytes: int64Pointer(0), Exclusions: []string{"small.bin", "boundary.bin", "link"}})
|
||||
if err != nil {
|
||||
t.Fatalf("checkFileSize() error = %v, want omitted gitlink size", err)
|
||||
}
|
||||
@@ -174,7 +174,7 @@ func TestCheckFileSizeUsesOnlyCandidateFilesIndividually(t *testing.T) {
|
||||
t.Fatal(err)
|
||||
}
|
||||
cleanupCandidateTree(t, tree)
|
||||
if err := checkFileSize(tree, &FileSizeSettings{LimitBytes: int64Pointer(10)}); err != nil {
|
||||
if err := checkFileSize(t.Context(), tree, &FileSizeSettings{LimitBytes: int64Pointer(10)}); err != nil {
|
||||
t.Fatalf("checkFileSize() error = %v, want current individual files to pass", err)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -98,7 +98,7 @@ func TestFileSizeExclusionsAreModuleRelative(t *testing.T) {
|
||||
}
|
||||
cleanupCandidateTree(t, tree)
|
||||
settings := fileSizeSettingsForModule(&FileSizeSettings{LimitBytes: int64Pointer(10), Exclusions: []string{"./dist/"}}, tree.modulePath)
|
||||
err = checkFileSize(tree, settings)
|
||||
err = checkFileSize(t.Context(), tree, settings)
|
||||
var failure *Failure
|
||||
if !errors.As(err, &failure) || failure.Check != FileSize || len(failure.Findings) != 1 || failure.Findings[0].Path != "nested/dist-old/output.bin" {
|
||||
t.Fatalf("module-relative file-size exclusion error = %v, want only nested/dist-old/output.bin", err)
|
||||
|
||||
@@ -48,10 +48,10 @@ func TestReadCandidateTreeUsesCapturedCommitAndEnumeratesGitEntries(t *testing.T
|
||||
t.Errorf("committed tree is missing %q", path)
|
||||
}
|
||||
}
|
||||
if got := entries["binary.dat"]; got.size != 4 || !got.hasSize {
|
||||
t.Errorf("binary metadata = %#v, want size 4", got)
|
||||
if got := entries["binary.dat"]; got.mode != "100644" || got.kind != "blob" {
|
||||
t.Errorf("binary metadata = %#v", got)
|
||||
}
|
||||
if got := entries["readme-link"]; got.mode != "120000" || got.kind != "blob" || !got.hasSize {
|
||||
if got := entries["readme-link"]; got.mode != "120000" || got.kind != "blob" {
|
||||
t.Errorf("symlink metadata = %#v", got)
|
||||
}
|
||||
if got := entries["submodule"]; got.mode != "160000" || got.kind != "commit" || got.hasSize {
|
||||
|
||||
@@ -190,7 +190,7 @@ func Run(ctx context.Context, request Request) (result error) {
|
||||
}
|
||||
}
|
||||
if request.Settings.FileSize != nil {
|
||||
if err := checkFileSize(tree, fileSizeSettings); err != nil {
|
||||
if err := checkFileSize(ctx, tree, fileSizeSettings); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
@@ -46,6 +46,59 @@ func TestSourceOperationsRejectUnavailableNoteWithoutTransport(t *testing.T) {
|
||||
assertInspectionStayedLocal(t, repository, attempts, before)
|
||||
}
|
||||
|
||||
func TestScopedValidationReadsOnlyRequiredBlobsInPartialClone(t *testing.T) {
|
||||
source := newPublishFixture(t)
|
||||
addNestedReleaseModule(t, source.work, "nested", "example.com/nested", "v1.2.3")
|
||||
writeFile(t, source.work, "nested/NOTICE.txt", "nested notice\n")
|
||||
writeFile(t, source.work, "nested/README.md", "# Nested guide\n")
|
||||
writeFile(t, source.work, "nested/go.work", "go 1.26.0\nuse .\n")
|
||||
writeFile(t, source.work, "docs/links.md", "# Links\n\n[Guide](../nested/README.md#nested-guide)\n")
|
||||
gitRun(t, source.work, "add", ".")
|
||||
gitRun(t, source.work, "commit", "-m", "Add independently scoped module")
|
||||
repository := newFilteredSparseCheckout(t, source.work,
|
||||
"/go.mod", "/fixture_test.go", "/race_fixture_test.go", "/source.txt", "/docs/")
|
||||
before := gitText(t, repository, "cat-file", "--batch-all-objects", "--batch-check=%(objectname)")
|
||||
for _, name := range []string{"nested/fixture_test.go", "nested/NOTICE.txt", "nested/README.md"} {
|
||||
object := gitText(t, repository, "rev-parse", "HEAD:"+name)
|
||||
if strings.Contains(before, object) {
|
||||
t.Fatalf("fixture already contains excluded blob %q", name)
|
||||
}
|
||||
}
|
||||
attempts := forbidInspectionTransport(t, repository)
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
selection releasekit.ValidationSpec
|
||||
wantCheck releasekit.ValidationCheck
|
||||
wantPath string
|
||||
}{
|
||||
{name: "formatting", selection: releasekit.ValidationSpec{GoFormat: true, SharedFiles: []string{}}},
|
||||
{name: "file sizes", selection: releasekit.ValidationSpec{FileSize: &releasekit.FileSizeValidationSpec{}, SharedFiles: []string{}}},
|
||||
{name: "workspace metadata", selection: releasekit.ValidationSpec{ModuleDistribution: releasekit.ModuleDistributionValidationSpec{WorkspaceFiles: true}, SharedFiles: []string{}}},
|
||||
{name: "explicit shared metadata", selection: releasekit.ValidationSpec{SharedFiles: []string{"nested/NOTICE.txt"}}},
|
||||
{name: "excluded shared size", selection: releasekit.ValidationSpec{FileSize: &releasekit.FileSizeValidationSpec{Exclusions: []string{"nested/NOTICE.txt"}}, SharedFiles: []string{"nested/NOTICE.txt"}}},
|
||||
{name: "required shared size", selection: releasekit.ValidationSpec{FileSize: &releasekit.FileSizeValidationSpec{}, SharedFiles: []string{"nested/NOTICE.txt"}}, wantCheck: "file-size", wantPath: "nested/NOTICE.txt"},
|
||||
{name: "required shared source", selection: releasekit.ValidationSpec{GoFormat: true, SharedFiles: []string{"nested/fixture_test.go"}}, wantCheck: "go-format", wantPath: "nested/fixture_test.go"},
|
||||
{name: "required reference", selection: releasekit.ValidationSpec{Markdown: &releasekit.MarkdownValidationSpec{}, SharedFiles: []string{}}, wantCheck: "markdown-links", wantPath: "nested/README.md"},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
_, err := releasekit.Check(t.Context(), releasekit.ReleaseRequest{
|
||||
RepositoryDir: repository, Product: "Sample App", Version: "v1.2.3", Validation: tc.selection,
|
||||
})
|
||||
if tc.wantCheck == "" {
|
||||
if err != nil {
|
||||
t.Fatalf("scoped validation failed: %v", err)
|
||||
}
|
||||
} else {
|
||||
var failure *releasekit.ValidationError
|
||||
if !errors.As(err, &failure) || failure.Check != tc.wantCheck || !strings.Contains(err.Error(), tc.wantPath) {
|
||||
t.Fatalf("validation error = %v; want %s inspection of %q", err, tc.wantCheck, tc.wantPath)
|
||||
}
|
||||
}
|
||||
assertInspectionStayedLocal(t, repository, attempts, before)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func newFilteredSparseCheckout(t *testing.T, source string, paths ...string) string {
|
||||
t.Helper()
|
||||
gitRun(t, source, "config", "uploadpack.allowFilter", "true")
|
||||
|
||||
Reference in New Issue
Block a user