PR#36: Check if fork exists before cloning - pag

pag

#36 Check if fork exists before cloning

Merged 5 years ago by lsedlar. Opened 5 years ago by lsedlar.

lsedlar/pag namespace-repo-url into develop

Check if fork exists before cloning

Lubomír Sedlář • 5 years ago

768543a

pag/commands/clone.py

file modified

+16 -1

		`@@ -1,4 +1,5 @@`
		`import click`
		`+ import requests`

		`from pag.app import app`
		`from pag.utils import (`
		`@@ -6,6 +7,12 @@`
		`run,`
		`)`

		`+ def _check_repo(namespace, name):`
		`+ """Check if a repo with this name exists in given namespace."""`
		`+ url = 'https://pagure.io/api/0/projects'`
		`+ response = requests.get(url, {'namespace': namespace, 'name': name})`
		`+ return bool(response.json()['projects'])`
		`+`

		`@app.command()`
		`@click.argument('name')`
		`@@ -15,6 +22,14 @@`
		`Clone an existing repo. Use the '-a' option when you don't have commit`
		`access to the repository.`
		`"""`
		`+`
		`+ force_no_fork = False`
		`+ if name.count('/') == 1:`
		`+ # There's exactly one slash in the name given. It could be fork or a`
		`+ # repo in namespace. We have no way of telling them apart other than`
		`+ # asking Pagure itself.`
		`+ force_no_fork = _check_repo(*name.split('/'))`
		`+`
		`use_ssh = False if anonymous else True`
		`- url = repo_url(name, ssh=use_ssh, git=True)`
		`+ url = repo_url(name, ssh=use_ssh, git=True, force_no_fork=force_no_fork)`
		`run(['git', 'clone', url, name.split('/')[-1]])`

pag/utils.py

file modified

+10 -4

		`@@ -91,19 +91,25 @@`
		`return branch`


		`- def repo_url(name, ssh=False, git=False, domain='pagure.io'):`
		`+ def repo_url(name, ssh=False, git=False, domain='pagure.io', force_no_fork=False):`
		`+ """Generate a URL to a project.`
		`+`
		`+ :param ssh: whether to use ssh or https protocol`
		`+ :param git: whether to append .git suffix`
		`+ :param domain: Pagure instance we are interested in`
		`+ :param force_no_fork: whether to check if the name could actually be a fork`
		`+ """`
		`if ssh:`
		`prefix = 'ssh://git@'`
		`else:`
		`prefix = 'https://'`

		`- if '/' in name:`
		`+ suffix = '%s' % name`
		`+ if not force_no_fork and '/' in name:`
		`if git:`
		`suffix = 'forks/%s' % name`
		`else:`
		`suffix = 'fork/%s' % name`
		`- else:`
		`- suffix = '%s' % name`

		`if git:`
		`suffix = suffix + '.git'`

tests/test_clone.py

file added

+102

		`@@ -0,0 +1,102 @@`
		`+ import unittest`
		`+ import mock`
		`+`
		`+ from click.testing import CliRunner`
		`+`
		`+ from pag.commands.clone import clone`
		`+`
		`+ EMPTY_RESPONSE = mock.Mock(json=lambda : {`
		`+ 'total_projects': 0,`
		`+ 'projects': [],`
		`+ })`
		`+`
		`+ RESPONSE_WITH_REPO = mock.Mock(json=lambda: {`
		`+ 'total_projects': 0,`
		`+ 'projects': [{'name': 'fedmod', 'namespace': 'modularity'}],`
		`+ })`
		`+`
		`+`
		`+ class CloneTest(unittest.TestCase):`
		`+ def setUp(self):`
		`+ self.runner = CliRunner()`
		`+ self.maxDiff = None`
		`+ self.patcher = mock.patch('requests.get')`
		`+ self.mock_get = self.patcher.start()`
		`+ self.mock_get.return_value = EMPTY_RESPONSE`
		`+`
		`+ def tearDown(self):`
		`+ self.patcher.stop()`
		`+`
		`+ @mock.patch('pag.commands.clone.run')`
		`+ def test_clone(self, run):`
		`+ result = self.runner.invoke(clone, ['pag'])`
		`+`
		`+ self.assertEqual(result.exit_code, 0)`
		`+ self.assertEqual(`
		`+ run.call_args_list,`
		`+ [mock.call(['git', 'clone', 'ssh://git@pagure.io/pag.git', 'pag'])]`
		`+ )`
		`+ self.assertEqual(self.mock_get.call_args_list, [])`
		`+`
		`+ @mock.patch('pag.commands.clone.run')`
		`+ def test_clone_anonymous(self, run):`
		`+ result = self.runner.invoke(clone, ['-a', 'pag'])`
		`+`
		`+ self.assertEqual(result.exit_code, 0)`
		`+ self.assertEqual(`
		`+ run.call_args_list,`
		`+ [mock.call(['git', 'clone', 'https://pagure.io/pag.git', 'pag'])]`
		`+ )`
		`+ self.assertEqual(self.mock_get.call_args_list, [])`
		`+`
		`+ @mock.patch('pag.commands.clone.run')`
		`+ def test_clone_fork(self, run):`
		`+ result = self.runner.invoke(clone, ['ralph/pag'])`
		`+`
		`+ self.assertEqual(result.exit_code, 0)`
		`+ self.assertEqual(`
		`+ run.call_args_list,`
		`+ [mock.call(['git', 'clone', 'ssh://git@pagure.io/forks/ralph/pag.git', 'pag'])]`
		`+ )`
		`+ self.assertEqual(self.mock_get.call_args_list,`
		`+ [mock.call(mock.ANY, {'namespace': 'ralph', 'name': 'pag'})])`
		`+`
		`+ @mock.patch('pag.commands.clone.run')`
		`+ def test_clone_fork_anonymous(self, run):`
		`+ result = self.runner.invoke(clone, ['-a', 'ralph/pag'])`
		`+`
		`+ self.assertEqual(result.exit_code, 0)`
		`+ self.assertEqual(`
		`+ run.call_args_list,`
		`+ [mock.call(['git', 'clone', 'https://pagure.io/forks/ralph/pag.git', 'pag'])]`
		`+ )`
		`+ self.assertEqual(self.mock_get.call_args_list,`
		`+ [mock.call(mock.ANY, {'namespace': 'ralph', 'name': 'pag'})])`
		`+`
		`+ @mock.patch('pag.commands.clone.run')`
		`+ def test_clone_namespace(self, run):`
		`+ self.mock_get.return_value = RESPONSE_WITH_REPO`
		`+`
		`+ result = self.runner.invoke(clone, ['modularity/fedmod'])`
		`+`
		`+ self.assertEqual(result.exit_code, 0)`
		`+ self.assertEqual(`
		`+ run.call_args_list,`
		`+ [mock.call(['git', 'clone', 'ssh://git@pagure.io/modularity/fedmod.git', 'fedmod'])]`
		`+ )`
		`+ self.assertEqual(self.mock_get.call_args_list,`
		`+ [mock.call(mock.ANY, {'namespace': 'modularity', 'name': 'fedmod'})])`
		`+`
		`+ @mock.patch('pag.commands.clone.run')`
		`+ def test_clone_namespace_anonymous(self, run):`
		`+ self.mock_get.return_value = RESPONSE_WITH_REPO`
		`+`
		`+ result = self.runner.invoke(clone, ['-a', 'modularity/fedmod'])`
		`+`
		`+ self.assertEqual(result.exit_code, 0)`
		`+ self.assertEqual(`
		`+ run.call_args_list,`
		`+ [mock.call(['git', 'clone', 'https://pagure.io/modularity/fedmod.git', 'fedmod'])]`
		`+ )`
		`+ self.assertEqual(self.mock_get.call_args_list,`
		`+ [mock.call(mock.ANY, {'namespace': 'modularity', 'name': 'fedmod'})])`

lsedlar commented 5 years ago

Current heuristic for determining forks is checking if there is a slash in the repo name. That however does not work in the face of namespaces.

This patch adds an option to repo_url function to force the code to treat the name as is and not as a fork. The caller will have to decide what to do.

In clone command we can query Pagure API to figure out if we are cloning a fork or repo in namespace. This incurs a small slowdown, but makes it work for both cases.

Fixes: https://pagure.io/pag/issue/27