From d26e87661baed9aabffa343b2c9ea4959a0927f9 Mon Sep 17 00:00:00 2001 From: Veiko Aasa Date: Sat, 11 Apr 2020 19:13:45 +0300 Subject: [PATCH] gitweb: Improve error handling when creating repository If an error occurs during creation of a git repository, delete possibly corrupted git repository directory and show more specific error message. Closes #1829 Tests performed: - Gitweb unit and functional tests pass - Create a small disk for git repositories: > dd if=/dev/zero of=disk.img iflag=fullblock bs=128k count=100 && sync > mkfs.ext4 disk.img > mount -o loop disk.img /var/lib/git/ - Clone a large repository https://salsa.debian.org/freedombox-team/plinth Disk got full during cloning remote repository. Repository listing do not show this repository anymore. (No errors is shown to the user.) - Fill disk space: > head -c 1G /var/lib/git/myfile - Disk is full. Cloning an existing remote repository fails with an error message (No space left on device) - Disk is full. Creating a new repository fails with an error message (No space left on device) Signed-off-by: Veiko Aasa Reviewed-by: James Valleroy --- actions/gitweb | 41 +++++++++++++++-------- plinth/modules/gitweb/tests/test_views.py | 10 +++--- plinth/modules/gitweb/views.py | 12 ++++--- 3 files changed, 41 insertions(+), 22 deletions(-) diff --git a/actions/gitweb b/actions/gitweb index 6feec93d0..023b2c269 100755 --- a/actions/gitweb +++ b/actions/gitweb @@ -156,8 +156,11 @@ def _clone_with_progress_report(url, repo_dir): if currenttime - starttime > 1: elapsed = _clone_status_line_to_percent(line) if elapsed is not None: - with open(status_file, 'w') as file_handle: - file_handle.write(elapsed) + try: + with open(status_file, 'w') as file_handle: + file_handle.write(elapsed) + except OSError as error: + errors.append(str(error)) starttime = currenttime @@ -183,12 +186,15 @@ def _prepare_clone_repo(arguments): repo_dir = os.path.join(GIT_REPO_PATH, repo_name) os.mkdir(repo_dir) - if arguments.is_private: - _set_access_status(repo_name, 'private') - status_file = os.path.join(repo_dir, 'clone_progress') - with open(status_file, 'w') as file_handle: - file_handle.write('0') + try: + if arguments.is_private: + _set_access_status(repo_name, 'private') + with open(status_file, 'w') as file_handle: + file_handle.write('0') + except OSError: + shutil.rmtree(repo_dir) + raise def _clone_status_line_to_percent(line): @@ -238,14 +244,21 @@ def _clone_repo(arguments): def _create_repo(arguments): """Create an empty repository.""" repo = arguments.name - subprocess.check_call(['git', 'init', '--bare', repo], cwd=GIT_REPO_PATH) - if not arguments.keep_ownership: - subprocess.check_call(['chown', '-R', 'www-data:www-data', repo], + try: + subprocess.check_call(['git', 'init', '--bare', repo], cwd=GIT_REPO_PATH) - _set_repo_description(repo, arguments.description) - _set_repo_owner(repo, arguments.owner) - if arguments.is_private: - _set_access_status(repo, 'private') + if not arguments.keep_ownership: + subprocess.check_call(['chown', '-R', 'www-data:www-data', repo], + cwd=GIT_REPO_PATH) + _set_repo_description(repo, arguments.description) + _set_repo_owner(repo, arguments.owner) + if arguments.is_private: + _set_access_status(repo, 'private') + except (subprocess.CalledProcessError, OSError): + repo_path = os.path.join(GIT_REPO_PATH, repo) + if os.path.isdir(repo_path): + shutil.rmtree(repo_path) + raise def _get_repo_description(repo): diff --git a/plinth/modules/gitweb/tests/test_views.py b/plinth/modules/gitweb/tests/test_views.py index 7980d8101..65678a8f1 100644 --- a/plinth/modules/gitweb/tests/test_views.py +++ b/plinth/modules/gitweb/tests/test_views.py @@ -146,8 +146,10 @@ def test_create_repo_invalid_name_view(rf): def test_create_repo_failed_view(rf): """Test that repo creation failure sends correct error message.""" - with patch('plinth.modules.gitweb.create_repo', - side_effect=ActionError('Error')): + general_error_message = "An error occurred while creating the repository." + error_description = 'some error' + with patch('plinth.modules.gitweb.create_repo', side_effect=ActionError( + 'gitweb', '', error_description)): form_data = { 'gitweb-name': 'something_other', 'gitweb-description': '', @@ -157,8 +159,8 @@ def test_create_repo_failed_view(rf): view = views.CreateRepoView.as_view() response, messages = make_request(request, view) - assert list(messages)[ - 0].message == 'An error occurred while creating the repository.' + assert list(messages)[0].message == '{0} {1}'.format( + general_error_message, error_description) assert response.status_code == 302 diff --git a/plinth/modules/gitweb/views.py b/plinth/modules/gitweb/views.py index 3c9c6d9f3..9affd6baf 100644 --- a/plinth/modules/gitweb/views.py +++ b/plinth/modules/gitweb/views.py @@ -60,11 +60,15 @@ class CreateRepoView(SuccessMessageMixin, FormView): try: gitweb.create_repo(form_data['name'], form_data['description'], form_data['owner'], form_data['is_private']) - except ActionError: + except ActionError as error: + self.success_message = '' + error_text = error.args[2].split('\n')[0] messages.error( - self.request, - _('An error occurred while creating the repository.')) - gitweb.app.update_service_access() + self.request, "{0} {1}".format( + _('An error occurred while creating the repository.'), + error_text)) + else: + gitweb.app.update_service_access() return super().form_valid(form)