From 768543a5bf6a2d60f4388d7bb72967e417afdc56 Mon Sep 17 00:00:00 2001 From: Lubomír Sedlář Date: Apr 22 2018 19:04:18 +0000 Subject: Check if fork exists before cloning 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 --- diff --git a/pag/commands/clone.py b/pag/commands/clone.py index 37e55a0..132b83f 100644 --- a/pag/commands/clone.py +++ b/pag/commands/clone.py @@ -1,4 +1,5 @@ import click +import requests from pag.app import app from pag.utils import ( @@ -6,6 +7,12 @@ from pag.utils import ( 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 @@ def clone(name, anonymous): 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]]) diff --git a/pag/utils.py b/pag/utils.py index be08f22..fcb1605 100644 --- a/pag/utils.py +++ b/pag/utils.py @@ -91,19 +91,25 @@ def get_current_local_branch(): 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' diff --git a/tests/test_clone.py b/tests/test_clone.py new file mode 100644 index 0000000..6281b5b --- /dev/null +++ b/tests/test_clone.py @@ -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'})])