From 796ff26e0e9de8dc746064e57db13518dedcf671 Mon Sep 17 00:00:00 2001 From: merlleu Date: Tue, 24 Oct 2023 05:26:38 +0200 Subject: [PATCH] Do not force creation of _cargo-index repo on publish (#27266) Hello there, Cargo Index over HTTP is now prefered over git for package updates: we should not force users who do not need the GIT repo to have the repo created/updated on each publish (it can still be created in the packages settings). The current behavior when publishing is to check if the repo exist and create it on the fly if not, then update it's content. Cargo HTTP Index does not rely on the repo itself so this will be useless for everyone not using the git protocol for cargo registry. This PR only disable the creation on the fly of the repo when publishing a crate. This is linked to #26844 (error 500 when trying to publish a crate if user is missing write access to the repo) because it's now optional. --------- Co-authored-by: KN4CK3R --- routers/api/packages/cargo/cargo.go | 4 ++-- services/packages/cargo/index.go | 12 +++++++++--- services/packages/cleanup/cleanup.go | 4 ++-- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/routers/api/packages/cargo/cargo.go b/routers/api/packages/cargo/cargo.go index 225b6b5ad..8f1e965c9 100644 --- a/routers/api/packages/cargo/cargo.go +++ b/routers/api/packages/cargo/cargo.go @@ -250,7 +250,7 @@ func UploadPackage(ctx *context.Context) { return } - if err := cargo_service.AddOrUpdatePackageIndex(ctx, ctx.Doer, ctx.Package.Owner, pv.PackageID); err != nil { + if err := cargo_service.UpdatePackageIndexIfExists(ctx, ctx.Doer, ctx.Package.Owner, pv.PackageID); err != nil { if err := packages_service.DeletePackageVersionAndReferences(ctx, pv); err != nil { log.Error("Rollback creation of package version: %v", err) } @@ -301,7 +301,7 @@ func yankPackage(ctx *context.Context, yank bool) { return } - if err := cargo_service.AddOrUpdatePackageIndex(ctx, ctx.Doer, ctx.Package.Owner, pv.PackageID); err != nil { + if err := cargo_service.UpdatePackageIndexIfExists(ctx, ctx.Doer, ctx.Package.Owner, pv.PackageID); err != nil { apiError(ctx, http.StatusInternalServerError, err) return } diff --git a/services/packages/cargo/index.go b/services/packages/cargo/index.go index 0561f168e..8164ffb01 100644 --- a/services/packages/cargo/index.go +++ b/services/packages/cargo/index.go @@ -106,10 +106,16 @@ func RebuildIndex(ctx context.Context, doer, owner *user_model.User) error { ) } -func AddOrUpdatePackageIndex(ctx context.Context, doer, owner *user_model.User, packageID int64) error { - repo, err := getOrCreateIndexRepository(ctx, doer, owner) +func UpdatePackageIndexIfExists(ctx context.Context, doer, owner *user_model.User, packageID int64) error { + // We do not want to force the creation of the repo here + // cargo http index does not rely on the repo itself, + // so if the repo does not exist, we just do nothing. + repo, err := repo_model.GetRepositoryByOwnerAndName(ctx, owner.Name, IndexRepositoryName) if err != nil { - return err + if errors.Is(err, util.ErrNotExist) { + return nil + } + return fmt.Errorf("GetRepositoryByOwnerAndName: %w", err) } p, err := packages_model.GetPackageByID(ctx, packageID) diff --git a/services/packages/cleanup/cleanup.go b/services/packages/cleanup/cleanup.go index 77bcfb194..9bdd9d6aa 100644 --- a/services/packages/cleanup/cleanup.go +++ b/services/packages/cleanup/cleanup.go @@ -110,8 +110,8 @@ func ExecuteCleanupRules(outerCtx context.Context) error { if err != nil { return fmt.Errorf("GetUserByID failed: %w", err) } - if err := cargo_service.AddOrUpdatePackageIndex(ctx, owner, owner, p.ID); err != nil { - return fmt.Errorf("CleanupRule [%d]: cargo.AddOrUpdatePackageIndex failed: %w", pcr.ID, err) + if err := cargo_service.UpdatePackageIndexIfExists(ctx, owner, owner, p.ID); err != nil { + return fmt.Errorf("CleanupRule [%d]: cargo.UpdatePackageIndexIfExists failed: %w", pcr.ID, err) } } }