From d69c0db4fc7becc2fcab2ef027e088b29140c032 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 8 Oct 2026 13:52:07 +0200 Subject: [PATCH] gitea: stop a real npm retention when a file was not read whole MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A lockfile that could not be read is one of its repository's files that could not be read (hq ADR 0251 §4): a version only it pins would be deleted while in use. A real run now refuses on files_not_read as it does on repositories_unread; a dry run still reports. --- modules/gitea/README.md | 5 ++-- modules/gitea/cmd/npm-registry/retention.go | 10 ++++++-- .../gitea/cmd/npm-registry/retention_test.go | 23 +++++++++++++++++++ 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/modules/gitea/README.md b/modules/gitea/README.md index 6ef48150..1cf89306 100644 --- a/modules/gitea/README.md +++ b/modules/gitea/README.md @@ -35,8 +35,9 @@ why under `keeps_all`. **Nothing is deleted on partial knowledge.** A repository whose files could not be listed, or a manifest or lockfile that could not be fetched, is named under `repositories_unread` in every answer, and a real run then deletes nothing at all. A file that was fetched but is not readable (broken JSON) is -named under `files_not_read`; what it would have named is not known, and the operator reads that list -before a real run. Under `node_modules` nothing is read: that is what a lockfile already says. An empty +named under `files_not_read`, and so is a lockfile entry of a shape the reader does not know (the rest of +that file is still read); what it would have named is not known, so a real run then deletes nothing at all +either. Under `node_modules` nothing is read: that is what a lockfile already says. An empty repository, or one without its default branch, holds nothing to read and is not a failure. A real run deletes each version through the forge's own interface (`DELETE diff --git a/modules/gitea/cmd/npm-registry/retention.go b/modules/gitea/cmd/npm-registry/retention.go index 8331bdcb..4f9915db 100644 --- a/modules/gitea/cmd/npm-registry/retention.go +++ b/modules/gitea/cmd/npm-registry/retention.go @@ -395,8 +395,8 @@ func retention(ctx context.Context, f *Forge, only string, keep int, dryRun bool s.Repositories, countVersions(s), len(s.Packages), mib(p.RegistryBytes), len(p.Candidates), mib(p.Bytes))) s.Said = append(s.Said, sizesSaid(s)...) if len(s.NotRead) > 0 { - s.Said = append(s.Said, fmt.Sprintf("%d files were not read whole (files_not_read): a version only they name is "+ - "kept by nothing here", len(s.NotRead))) + s.Said = append(s.Said, fmt.Sprintf("%d files were not read whole (files_not_read): what they name is not "+ + "known, so a real run deletes nothing", len(s.NotRead))) } if len(s.Unread) > 0 { s.Said = append(s.Said, fmt.Sprintf("%d repositories or files could not be read: what they depend on is not "+ @@ -410,6 +410,12 @@ func retention(ctx context.Context, f *Forge, only string, keep int, dryRun bool return p, fmt.Errorf("refused: %d repositories or files could not be read, so what depends on what is not "+ "known; nothing was deleted (first: %s)", len(s.Unread), s.Unread[0]) } + // A file that was fetched but not read whole is a file of its repository that could not be read + // (novox/hq ADR 0251 §4): a version only it names would be deleted while in use. + if len(s.NotRead) > 0 { + return p, fmt.Errorf("refused: %d files were not read whole, so what they name is not known; nothing was "+ + "deleted (first: %s)", len(s.NotRead), s.NotRead[0]) + } del, cancel := context.WithTimeout(context.Background(), budget) defer cancel() for _, c := range p.Candidates { diff --git a/modules/gitea/cmd/npm-registry/retention_test.go b/modules/gitea/cmd/npm-registry/retention_test.go index 545d5c2d..1b0fdeaf 100644 --- a/modules/gitea/cmd/npm-registry/retention_test.go +++ b/modules/gitea/cmd/npm-registry/retention_test.go @@ -465,3 +465,26 @@ func TestALockfileOfEveryShapeKeepsWhatItPins(t *testing.T) { t.Fatalf("0.2.0: %+v", got) } } + +func TestAnUnreadableLockfileStopsARealRunBeforeAnythingIsDeleted(t *testing.T) { + f := aRegistry(t) + f.repos["acme/odd"] = map[string]string{"package-lock.json": `{"lockfileVersion":3,"packages":{`} + p, err := retention(context.Background(), f.serve(), "", 3, false, "tidy") + if err == nil || !strings.Contains(err.Error(), "nothing was deleted") || !strings.Contains(err.Error(), "acme/odd:package-lock.json") { + t.Fatalf("got %v", err) + } + if len(f.deleted) != 0 { + t.Fatalf("deleted %v while a lockfile was not read", f.deleted) + } + if p == nil || len(p.NotRead) != 1 || len(p.Deleted) != 0 { + t.Fatalf("plan %+v", p) + } + // And the dry run says so, without failing, and still says what would go. + dry, err := retention(context.Background(), f.serve(), "", 3, true, "") + if err != nil || len(dry.NotRead) != 1 || len(dry.Candidates) == 0 { + t.Fatalf("dry run: %v %+v", err, dry) + } + if !strings.Contains(strings.Join(dry.Said, "\n"), "1 files were not read whole") { + t.Errorf("said %v", dry.Said) + } +}