PR#1901: Add a feature to ignore existing git repo on disk - pagure

pagure

#1901 Add a feature to ignore existing git repo on disk

Merged 7 years ago by pingou. Opened 7 years ago by pingou.

ignore_existing into master

Add a feature to ignore existing git repo on disk

Pierre-Yves Chibon • 7 years ago

abae2b7

pagure/lib/__init__.py

file modified

+41 -24

		`@@ -1193,7 +1193,8 @@`
		`gitfolder, docfolder, ticketfolder, requestfolder,`
		`description=None, url=None, avatar_email=None,`
		`parent_id=None, add_readme=False, userobj=None,`
		`- prevent_40_chars=False, namespace=None, user_ns=False):`
		`+ prevent_40_chars=False, namespace=None, user_ns=False,`
		`+ ignore_existing_repo=False):`
		`''' Create a new project based on the information provided.`
		`'''`
		`if name in blacklist or (`
		`@@ -1233,10 +1234,20 @@`
		`if namespace:`
		`path = '%s/%s' % (namespace, name)`

		`+ # Repo exists on disk`
		`gitrepo = os.path.join(gitfolder, '%s.git' % path)`
		`if os.path.exists(gitrepo):`
		`+ if not ignore_existing_repo:`
		`+ raise pagure.exceptions.RepoExistsException(`
		`+ 'The project repo "%s" already exists' % path`
		`+ )`
		`+`
		`+ # Repo exists in the DB`
		`+ repo = pagure.lib.get_project(session, name, namespace=namespace)`
		`+ if repo:`
		`raise pagure.exceptions.RepoExistsException(`
		`- 'The project repo "%s" already exists' % path`
		`+ 'The project repo "%s/%s" already exists in the database' % (`
		`+ namespace, name)`
		`)`

		`project = model.Project(`
		`@@ -1285,34 +1296,40 @@`

		`docrepo = os.path.join(docfolder, project.path)`
		`if os.path.exists(docrepo):`
		`- shutil.rmtree(gitrepo)`
		`- raise pagure.exceptions.RepoExistsException(`
		`- 'The docs repo "%s" already exists' % project.path`
		`- )`
		`- pygit2.init_repository(docrepo, bare=True)`
		`+ if not ignore_existing_repo:`
		`+ shutil.rmtree(gitrepo)`
		`+ raise pagure.exceptions.RepoExistsException(`
		`+ 'The docs repo "%s" already exists' % project.path`
		`+ )`
		`+ else:`
		`+ pygit2.init_repository(docrepo, bare=True)`

		`ticketrepo = os.path.join(ticketfolder, project.path)`
		`if os.path.exists(ticketrepo):`
		`- shutil.rmtree(gitrepo)`
		`- shutil.rmtree(docrepo)`
		`- raise pagure.exceptions.RepoExistsException(`
		`- 'The tickets repo "%s" already exists' % project.path`
		`- )`
		`- pygit2.init_repository(`
		`- ticketrepo, bare=True,`
		`- mode=pygit2.C.GIT_REPOSITORY_INIT_SHARED_GROUP)`
		`+ if not ignore_existing_repo:`
		`+ shutil.rmtree(gitrepo)`
		`+ shutil.rmtree(docrepo)`
		`+ raise pagure.exceptions.RepoExistsException(`
		`+ 'The tickets repo "%s" already exists' % project.path`
		`+ )`
		`+ else:`
		`+ pygit2.init_repository(`
		`+ ticketrepo, bare=True,`
		`+ mode=pygit2.C.GIT_REPOSITORY_INIT_SHARED_GROUP)`

		`requestrepo = os.path.join(requestfolder, project.path)`
		`if os.path.exists(requestrepo):`
		`- shutil.rmtree(gitrepo)`
		`- shutil.rmtree(docrepo)`
		`- shutil.rmtree(ticketrepo)`
		`- raise pagure.exceptions.RepoExistsException(`
		`- 'The requests repo "%s" already exists' % project.path`
		`- )`
		`- pygit2.init_repository(`
		`- requestrepo, bare=True,`
		`- mode=pygit2.C.GIT_REPOSITORY_INIT_SHARED_GROUP)`
		`+ if not ignore_existing_repo:`
		`+ shutil.rmtree(gitrepo)`
		`+ shutil.rmtree(docrepo)`
		`+ shutil.rmtree(ticketrepo)`
		`+ raise pagure.exceptions.RepoExistsException(`
		`+ 'The requests repo "%s" already exists' % project.path`
		`+ )`
		`+ else:`
		`+ pygit2.init_repository(`
		`+ requestrepo, bare=True,`
		`+ mode=pygit2.C.GIT_REPOSITORY_INIT_SHARED_GROUP)`

		`# Install the default hook`
		`plugin = pagure.lib.plugins.get_plugin('default')`

tests/test_pagure_lib.py

file modified

+45 -1

		`@@ -937,6 +937,51 @@`
		`self.assertTrue(os.path.exists(ticketrepo))`
		`self.assertTrue(os.path.exists(requestrepo))`

		`+ # Try re-creating it ignoring the existing repos - but repo in the DB`
		`+ self.assertRaises(`
		`+ pagure.exceptions.PagureException,`
		`+ pagure.lib.new_project,`
		`+ session=self.session,`
		`+ user='pingou',`
		`+ name='testproject',`
		`+ blacklist=[],`
		`+ allowed_prefix=[],`
		`+ gitfolder=gitfolder,`
		`+ docfolder=docfolder,`
		`+ ticketfolder=ticketfolder,`
		`+ requestfolder=requestfolder,`
		`+ description='description for testproject',`
		`+ parent_id=None`
		`+ )`
		`+ self.session.rollback()`
		`+`
		`+ # Re-create it, ignoring the existing repos on disk`
		`+ repo = pagure.lib.get_project(self.session, 'testproject')`
		`+ self.session.delete(repo)`
		`+ self.session.commit()`
		`+`
		`+ msg = pagure.lib.new_project(`
		`+ session=self.session,`
		`+ user='pingou',`
		`+ name='testproject',`
		`+ blacklist=[],`
		`+ allowed_prefix=[],`
		`+ gitfolder=gitfolder,`
		`+ docfolder=docfolder,`
		`+ ticketfolder=ticketfolder,`
		`+ requestfolder=requestfolder,`
		`+ description='description for testproject',`
		`+ parent_id=None,`
		`+ ignore_existing_repo=True`
		`+ )`
		`+ self.session.commit()`
		`+ self.assertEqual(msg, 'Project "testproject" created')`
		`+`
		`+ self.assertTrue(os.path.exists(gitrepo))`
		`+ self.assertTrue(os.path.exists(docrepo))`
		`+ self.assertTrue(os.path.exists(ticketrepo))`
		`+ self.assertTrue(os.path.exists(requestrepo))`
		`+`
		`# Drop the main git repo and try again`
		`shutil.rmtree(gitrepo)`
		`self.assertRaises(`
		`@@ -1027,7 +1072,6 @@`
		`'Project "pingou/ssssssssssssssssssssssssssssssssssssssss" '`
		`'created')`

		`-`
		`def test_new_project_user_ns(self):`
		`""" Test the new_project of pagure.lib with user_ns on. """`
		`gitfolder = os.path.join(self.path, 'repos')`

pingou commented 7 years ago

In the traditional use of pagure, if a repo exists on disk we want to not
create the project, but when using pagure as a front-end for a system
that handles creating the project on disk via a different process, in that
situation we want to be able to create the project in the DB while ignoring
the fact that there is/are already git repos on disk.

The flag ignore_existing_repo added to pagure.lib.new_project() will
allow that.

Add corresponding tests

rebased

7 years ago

vivekanand1101 commented 7 years ago

:thumbsup:

jcline commented 7 years ago

Personally, I'd break that test off into its own test case, but that's not a blocker. Both the change and the test look good :thumbsup:.

pingou commented 7 years ago

The nature of the change and the impact it has is such that I want to proceed as is, however, I have made a ticket to track this piece of debt that we will have to re-pay one day: https://pagure.io/pagure/issue/1903 :)

Thanks for the reviews !!

Pull-Request has been merged by pingou

7 years ago