Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Template repo files should be cleaned up after an error #28144

Open
jtran opened this issue Nov 21, 2023 · 1 comment · May be fixed by #31035
Open

Template repo files should be cleaned up after an error #28144

jtran opened this issue Nov 21, 2023 · 1 comment · May be fixed by #31035
Labels
issue/critical This issue should be fixed ASAP. If it is a PR, the PR should be merged ASAP type/bug

Comments

@jtran
Copy link
Contributor

jtran commented Nov 21, 2023

Description

Steps to reproduce:

  1. Create a repo from a template repo
  2. Create fails due to some reason (repo-avatar file not found, in my case)
  3. 500 error
  4. Go back in the browser to the new repository form
  5. Try to create an empty repo with the same name and owner (without the problematic template)

I expected the creation of an empty repo to work for a repo name that isn't used. However, the repo creation fails with an error message:

Files already exist for this repository. Contact the system administrator.

To fix it, the admin needs to delete the bare git repo from the filesystem using rm -r. This makes the repo name available again.

In this case, one cause is the original file not found. However, filesystem changes like creating the bare git repo should get reverted in the case of an error, the same way that database changes are reverted using a transaction.

Here is the database transaction that reverts db changes. As far as I know, there is no equivalent clean-up for the filesystem.

if err = db.WithTx(ctx, func(ctx context.Context) error {

Contrast this with CreateRepositoryDirectly() (not from a repo template), which cleans up.

if rollbackRepo != nil {
if errDelete := DeleteRepositoryDirectly(ctx, doer, rollbackRepo.ID); errDelete != nil {
log.Error("Rollback deleteRepository: %v", errDelete)

However, it's unclear to me that the above code calling DeleteRepositoryDirectly() (first introduced in #14384) actually cleans up the files because it first looks for the repo in the database, and if it's not there, returns early. At the point it's being called, the repo wouldn't be in the database because the transaction has been rolled back.

Gitea Version

1.20.5

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

No response

Screenshots

No response

Git Version

No response

Operating System

Linux

How are you running Gitea?

docker

Database

PostgreSQL

@jtran jtran added the type/bug label Nov 21, 2023
@lunny lunny added the issue/critical This issue should be fixed ASAP. If it is a PR, the PR should be merged ASAP label Nov 21, 2023
@disciple-dev
Copy link

Encountered this yesterday, glad to see someone already reported it! And thanks for the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue/critical This issue should be fixed ASAP. If it is a PR, the PR should be merged ASAP type/bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants