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) + } +}