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

Delete local branch when repo branch is deleted #6497

Merged
merged 6 commits into from Apr 8, 2019

Conversation

7 participants
@jolheiser
Copy link
Member

commented Apr 2, 2019

Fixes #6490

Currently, when creating a branch from the web UI (not a commit, directly from the branch dropdown), Gitea creates the branch in its local cache and pushes it to the real repo.

When the branch is deleted, it was not being deleted from the local cache and if a branch of the same name is created, Gitea panics because the local repo already has a branch with that name.

Since the Gitea's tmp directory is deleted on startup, the only (current) way to clear the local repo cache is to restart Gitea.

Delete local branch if it exists
Signed-off-by: jolheiser <john.olheiser@gmail.com>
@codecov-io

This comment has been minimized.

Copy link

commented Apr 2, 2019

Codecov Report

❗️ No coverage uploaded for pull request base (master@49b2f45). Click here to learn what that means.
The diff coverage is 25%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #6497   +/-   ##
=========================================
  Coverage          ?   40.41%           
=========================================
  Files             ?      404           
  Lines             ?    54140           
  Branches          ?        0           
=========================================
  Hits              ?    21883           
  Misses            ?    29240           
  Partials          ?     3017
Impacted Files Coverage Δ
routers/repo/branch.go 56.9% <0%> (ø)
models/repo_branch.go 54.97% <30%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 49b2f45...c569312. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 label Apr 2, 2019

@zeripath
Copy link
Contributor

left a comment

The local cache needs to go. Do we even need it still?

I cannot believe that we're actually requiring a full clone to just do a branch creation - particularly a full clone we keep around.

Can't we follow the technique in

https://github.com/go-gitea/gitea/blob/master/modules/uploader/update.go

Where we just do a thin clone, create an index from the appropriate old branch and then push it as the new branch. We probably don't even need an index - We could just push our HEAD to the new branch.

In fact I wouldn't be surprised if we couldn't just do a push from an empty place to create a new branch. Would have to look at the documentation for git push.

A branch is simply a file with a hash to a tree object. It's nothing magical. The only benefit with pushing is that you make sure hooks etc will run.

@jolheiser

This comment has been minimized.

Copy link
Member Author

commented Apr 2, 2019

If I do follow how the uploader does it, should I take that code and put it somewhere else (perhaps a new module, or in util) that can be re-used when we need to make a "temp" repo that gets cleaned up after use?

I could just use the existing code, but it would seem odd to use the uploader package when referencing temp repos.

@zeripath
Copy link
Contributor

left a comment

I'm gonna approve as this is a bug fix, but I'm gonna look at removing this unnecessary cloning from the codebase.

@GiteaBot GiteaBot added lgtm/need 1 and removed lgtm/need 2 labels Apr 3, 2019

@lafriks lafriks added the kind/bug label Apr 4, 2019

@lafriks lafriks added this to the 1.9.0 milestone Apr 4, 2019

@zeripath

This comment has been minimized.

Copy link
Contributor

commented Apr 6, 2019

Take a look at: #6505

@ngourdon

This comment has been minimized.

Copy link
Contributor

commented Apr 6, 2019

@zeripath Even if it's out of scope of this PR, yes you can push a new branch without checking it out.

The form of the git push command which will permit to do so is
git push <repository> <refspec>
repository can be a name like "origin" but may also be a path like "/path/to/gitea-repositories/username/repo.git".

From the directory of the gitea bare repository (/path/to/gitea-repositories/username/repo.git) you can do
git push . refs/heads/existing-branch:refs/heads/new-branch
or from a sha1
git push . sha1:refs/heads/new-branch

This will trigger git hooks

@lafriks

lafriks approved these changes Apr 6, 2019

@GiteaBot GiteaBot added lgtm/done and removed lgtm/need 1 labels Apr 6, 2019

lafriks added some commits Apr 6, 2019

@lafriks lafriks merged commit aa02463 into go-gitea:master Apr 8, 2019

2 checks passed

approvals/lgtm this commit looks good
continuous-integration/drone/pr Build is passing
Details

@jolheiser jolheiser deleted the jolheiser:6490_delete_local_branch branch Apr 8, 2019

@jolheiser

This comment has been minimized.

Copy link
Member Author

commented Apr 8, 2019

@zeripath Once your PR is merged I will look at submitting a new PR implementing those changes in this area. (Unless your PR just removes these changes as part of the conversion)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.