Don't automatically delete repository files if they are present #12409

zeripath · 2020-08-02T20:14:25Z

Prior to this PR Gitea would delete any repository files if they are
present during creation or migration. This can in certain circumstances
lead to data-loss and is slightly unpleasant.

This PR provides a mechanism for Gitea to adopt repositories on creation
and otherwise requires an explicit flag for deletion.

PushCreate is slightly different - the create will cause adoption if
that is allowed otherwise it will delete the data if that is allowed.

Signed-off-by: Andrew Thornton art27@cantab.net

Prior to this PR Gitea would delete any repository files if they are present during creation or migration. This can in certain circumstances lead to data-loss and is slightly unpleasant. This PR provides a mechanism for Gitea to adopt repositories on creation and otherwise requires an explicit flag for deletion. PushCreate is slightly different - the create will cause adoption if that is allowed otherwise it will delete the data if that is allowed. Signed-off-by: Andrew Thornton <art27@cantab.net>

Signed-off-by: Andrew Thornton <art27@cantab.net>

modules/repository/create.go

Signed-off-by: Andrew Thornton <art27@cantab.net>

modules/repository/create.go

lunny · 2020-08-04T08:41:16Z

How about not give the option to overwrite the exist repository and just returned error?

Only offer to adopt or overwrite if the user can do that. Allow the site administrator to adopt or overwrite in all circumstances Signed-off-by: Andrew Thornton <art27@cantab.net>

zeripath · 2020-08-04T09:58:10Z

OK I've gone one step further - the adopt or overwrite option is only displayed if it is permitted for that user. Otherwise just the error is returned. The site admin can always adopt or overwrite.

Signed-off-by: Andrew Thornton <art27@cantab.net>

codecov-commenter · 2020-09-06T11:39:16Z

Codecov Report

Merging #12409 into master will decrease coverage by 0.04%.
The diff coverage is 41.86%.

@@            Coverage Diff             @@
##           master   #12409      +/-   ##
==========================================
- Coverage   43.07%   43.02%   -0.05%     
==========================================
  Files         658      658              
  Lines       72454    72591     +137     
==========================================
+ Hits        31211    31234      +23     
- Misses      36186    36295     +109     
- Partials     5057     5062       +5

Impacted Files	Coverage Δ
models/error.go	`34.51% <0.00%> (-0.30%)`	⬇️
models/repo_generate.go	`12.50% <ø> (ø)`
modules/auth/repo_form.go	`42.34% <ø> (ø)`
modules/migrations/gitea.go	`6.86% <0.00%> (-0.02%)`	⬇️
modules/repository/generate.go	`0.00% <0.00%> (ø)`
modules/structs/repo.go	`50.00% <ø> (ø)`
routers/repo/setting.go	`14.67% <0.00%> (-0.29%)`	⬇️
services/repository/generate.go	`0.00% <0.00%> (ø)`
modules/repository/fork.go	`47.61% <6.66%> (-12.39%)`	⬇️
modules/repository/init.go	`50.95% <11.11%> (-8.00%)`	⬇️
... and 14 more

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 f215e01...74831a7. Read the comment docs.

Signed-off-by: Andrew Thornton <art27@cantab.net>

models/repo.go

lafriks · 2020-09-16T14:48:34Z

Please resolve conflicts

6543 · 2020-09-17T18:09:37Z

swagger lint

Signed-off-by: Andrew Thornton <art27@cantab.net>

zeripath · 2020-09-17T18:22:07Z

OK Swagger just needs to be run as part of our generate. Why aren't we doing that?

Signed-off-by: Andrew Thornton <art27@cantab.net>

zeripath · 2020-09-17T18:50:27Z

I'm serious - I don't understand why the swagger generate can't be done on generate. We can ensure that we generate a valid swagger description as part of lint - but AFAICS there is no good reason to have it in master.

lunny · 2020-09-17T23:56:04Z

modules/migrations/gitea.go

+			IsPrivate:            opts.Private,
+			IsMirror:             opts.Mirror,
+			Status:               models.RepositoryBeingMigrated,
+			OverwritePreExisting: opts.OverwritePreExisting && (g.doer.IsAdmin || setting.Repository.AllowOverwriteOfUnadoptedRepositories),


Why an admin could not follow the setting?

? This allows Site Admins to explicitly overwrite pre-existing repositories.

modules/repository/init.go

lunny · 2020-09-18T00:05:05Z

I don't like this PR now because I think it makes the process more complicated. Maybe we should add a new feature which is only a button adopt projects, then it will scan all the repositories root folder and automatically check if it exists on the database, if not then create it. If it's scanning, the button could not be clicked. Of course, the button should be in admin panel.

For this PR we can just return the error if the repository folder exists.

Signed-off-by: Andrew Thornton <art27@cantab.net>

zeripath · 2020-09-18T19:55:48Z

Any UI that involves adopting repositories is going to have to provide the information like:

Visibility
Description
Issue labels
Default branch

it's then going to require a UI to search to find repositories to adopt / overwrite.

lunny · 2020-09-19T07:02:28Z

You can change them after adopting them. Then you have resued the previous UI and features. And also this will resolve batching adopt projects feature.

6543

lunny's suggestion sounds resonable

Signed-off-by: Andrew Thornton <art27@cantab.net>

jolheiser · 2020-09-23T17:49:03Z

Under the context that this is primarily intended to stop unnecessary data loss (adoption/overwriting being a secondary effect of that), I think this PR is great.

I think the code it introduces could be used in a later PR to allow for batch adoption of repositories on disk.

6543

this is nice now :)

zeripath added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Aug 2, 2020

zeripath added this to the 1.13.0 milestone Aug 2, 2020

Update swagger

2fd0292

Signed-off-by: Andrew Thornton <art27@cantab.net>

a1012112796 reviewed Aug 3, 2020

View reviewed changes

modules/repository/create.go Outdated Show resolved Hide resolved

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 3, 2020

Fix tests and migrate overwrite

bd1d182

Signed-off-by: Andrew Thornton <art27@cantab.net>

lunny reviewed Aug 4, 2020

View reviewed changes

modules/repository/create.go Show resolved Hide resolved

as per @lunny

5075257

Only offer to adopt or overwrite if the user can do that. Allow the site administrator to adopt or overwrite in all circumstances Signed-off-by: Andrew Thornton <art27@cantab.net>

zeripath mentioned this pull request Aug 5, 2020

Mass import existing repositories from file system #11473

Closed

7 tasks

zeripath added 2 commits September 6, 2020 12:01

Merge remote-tracking branch 'origin/master' into adopt-repositories

e8fa2e6

Use setting.Repository.DefaultBranch for the default branch

b06c510

Signed-off-by: Andrew Thornton <art27@cantab.net>

Always set setting.Repository.DefaultBranch

d885a69

Signed-off-by: Andrew Thornton <art27@cantab.net>

6543 approved these changes Sep 6, 2020

View reviewed changes

GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Sep 6, 2020

lunny reviewed Sep 10, 2020

View reviewed changes

models/repo.go Show resolved Hide resolved

Merge remote-tracking branch 'origin/master' into adopt-repositories

45a6873

update swagger

608031d

Signed-off-by: Andrew Thornton <art27@cantab.net>

update templates

ca482b7

Signed-off-by: Andrew Thornton <art27@cantab.net>

mrsdizzie approved these changes Sep 17, 2020

View reviewed changes

GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Sep 17, 2020

Merge branch 'master' into adopt-repositories

4c143ff

lunny reviewed Sep 17, 2020

View reviewed changes

modules/repository/init.go Show resolved Hide resolved

ensure repo closed

ee66a29

Signed-off-by: Andrew Thornton <art27@cantab.net>

6543 requested changes Sep 19, 2020

View reviewed changes

zeripath added 4 commits September 19, 2020 14:40

Merge remote-tracking branch 'origin/master' into adopt-repositories

d74f3c9

Merge remote-tracking branch 'origin/master' into adopt-repositories

a08537b

Only show Adoption or Overwrite options if they're relevant

37aa668

Signed-off-by: Andrew Thornton <art27@cantab.net>

Only show Adoption or Overwrite options if they're relevant

969ee4b

Signed-off-by: Andrew Thornton <art27@cantab.net>

zeripath mentioned this pull request Sep 21, 2020

Adopt repositories #12920

Merged

zeripath added 2 commits September 23, 2020 18:11

Merge remote-tracking branch 'origin/master' into adopt-repositories

6a8ec83

fix broken div

74831a7

Signed-off-by: Andrew Thornton <art27@cantab.net>

6543 approved these changes Sep 23, 2020

View reviewed changes

zeripath closed this Sep 24, 2020

zeripath deleted the adopt-repositories branch September 25, 2020 17:01

go-gitea locked and limited conversation to collaborators Nov 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't automatically delete repository files if they are present #12409

Don't automatically delete repository files if they are present #12409

zeripath commented Aug 2, 2020

lunny commented Aug 4, 2020

zeripath commented Aug 4, 2020 •

edited

Loading

codecov-commenter commented Sep 6, 2020 •

edited

Loading

lafriks commented Sep 16, 2020

6543 commented Sep 17, 2020

zeripath commented Sep 17, 2020

zeripath commented Sep 17, 2020 •

edited

Loading

lunny Sep 17, 2020

zeripath Sep 18, 2020

lunny commented Sep 18, 2020 •

edited

Loading

zeripath commented Sep 18, 2020

lunny commented Sep 19, 2020

6543 left a comment

jolheiser commented Sep 23, 2020

6543 left a comment

Don't automatically delete repository files if they are present #12409

Don't automatically delete repository files if they are present #12409

Conversation

zeripath commented Aug 2, 2020

lunny commented Aug 4, 2020

zeripath commented Aug 4, 2020 • edited Loading

codecov-commenter commented Sep 6, 2020 • edited Loading

Codecov Report

lafriks commented Sep 16, 2020

6543 commented Sep 17, 2020

zeripath commented Sep 17, 2020

zeripath commented Sep 17, 2020 • edited Loading

lunny Sep 17, 2020

Choose a reason for hiding this comment

zeripath Sep 18, 2020

Choose a reason for hiding this comment

lunny commented Sep 18, 2020 • edited Loading

zeripath commented Sep 18, 2020

lunny commented Sep 19, 2020

6543 left a comment

Choose a reason for hiding this comment

jolheiser commented Sep 23, 2020

6543 left a comment

Choose a reason for hiding this comment

zeripath commented Aug 4, 2020 •

edited

Loading

codecov-commenter commented Sep 6, 2020 •

edited

Loading

zeripath commented Sep 17, 2020 •

edited

Loading

lunny commented Sep 18, 2020 •

edited

Loading