Git Backend: Use actual author, commit on upload
* Restructure AddonGitRepository so that init doesn't have any side-effects. * Add waffle + code and tests to create and commit new uploads to git. * Apply the actual FileUpload user to the git commit Fixes #8753 Fixes #9752
This commit is contained in:
Christopher Grebs
Родитель
2e692c5d6f
Коммит
1ae9e2c3c2
|
|
@ -8,6 +8,7 @@ import pygit2
|
|||
|
||||
from django.conf import settings
|
||||
from django.utils import translation
|
||||
from django.utils.functional import cached_property
|
||||
|
||||
import olympia.core.logger
|
||||
|
||||
|
|
@ -69,17 +70,19 @@ class AddonGitRepository(object):
|
|||
str(addon_id),
|
||||
package_type)
|
||||
|
||||
@cached_property
|
||||
def git_repository(self):
|
||||
if not os.path.exists(self.git_repository_path):
|
||||
os.makedirs(self.git_repository_path)
|
||||
self.git_repository = pygit2.init_repository(
|
||||
git_repository = pygit2.init_repository(
|
||||
path=self.git_repository_path,
|
||||
bare=False)
|
||||
# Write first commit to 'master' to act as HEAD
|
||||
tree = self.git_repository.TreeBuilder().write()
|
||||
self.git_repository.create_commit(
|
||||
git_repository.create_commit(
|
||||
'HEAD', # ref
|
||||
self.get_author(), # author
|
||||
self.get_author(), # commitor
|
||||
self.get_author(), # author, using addons-robot
|
||||
self.get_author(), # commiter, using addons-robot
|
||||
'Initializing repository', # message
|
||||
tree, # tree
|
||||
[]) # parents
|
||||
|
|
@ -87,10 +90,12 @@ class AddonGitRepository(object):
|
|||
log.debug('Initialized git repository {path}'.format(
|
||||
path=self.git_repository_path))
|
||||
else:
|
||||
self.git_repository = pygit2.Repository(self.git_repository_path)
|
||||
git_repository = pygit2.Repository(self.git_repository_path)
|
||||
|
||||
return git_repository
|
||||
|
||||
@classmethod
|
||||
def extract_and_commit_from_file_obj(cls, file_obj, channel):
|
||||
def extract_and_commit_from_file_obj(cls, file_obj, channel, author=None):
|
||||
"""Extract all files from `file_obj` and comit them.
|
||||
|
||||
This is doing the following:
|
||||
|
|
@ -188,7 +193,11 @@ class AddonGitRepository(object):
|
|||
|
||||
oid = worktree.repo.create_commit(
|
||||
None,
|
||||
repo.get_author(), repo.get_author(),
|
||||
# author, using the actual uploading user
|
||||
repo.get_author(author),
|
||||
# committer, using addons-robot because that's the user
|
||||
# actually doing the commit.
|
||||
repo.get_author(), # commiter, using addons-robot
|
||||
message,
|
||||
tree,
|
||||
# Set the current branch HEAD as the parent of this commit
|
||||
|
|
@ -208,10 +217,14 @@ class AddonGitRepository(object):
|
|||
file_obj.version.update(git_hash=commit.hex)
|
||||
return repo
|
||||
|
||||
def get_author(self):
|
||||
return pygit2.Signature(
|
||||
name='Mozilla Add-ons Robot',
|
||||
email='addons-dev-automation+github@mozilla.com')
|
||||
def get_author(self, user=None):
|
||||
if user is not None:
|
||||
author_name = user.name
|
||||
author_email = user.email
|
||||
else:
|
||||
author_name = 'Mozilla Add-ons Robot'
|
||||
author_email = 'addons-dev-automation+github@mozilla.com'
|
||||
return pygit2.Signature(name=author_name, email=author_email)
|
||||
|
||||
def find_or_create_branch(self, name):
|
||||
"""Lookup or create the branch named `name`"""
|
||||
|
|
|
|||
|
|
@ -6,18 +6,17 @@ import pytest
|
|||
from django.conf import settings
|
||||
|
||||
from olympia import amo
|
||||
from olympia.amo.tests import addon_factory, version_factory
|
||||
from olympia.amo.tests import addon_factory, version_factory, user_factory
|
||||
from olympia.lib.git import AddonGitRepository, TemporaryWorktree
|
||||
|
||||
|
||||
def test_temporary_worktree():
|
||||
repo = AddonGitRepository(1)
|
||||
|
||||
env = {'GIT_DIR': os.path.join(repo.git_repository_path, '.git')}
|
||||
env = {'GIT_DIR': repo.git_repository.path}
|
||||
|
||||
output = subprocess.check_output('git worktree list', shell=True, env=env)
|
||||
assert output.startswith(repo.git_repository_path)
|
||||
assert output.endswith('[master]\n')
|
||||
assert output.startswith(repo.git_repository.path)
|
||||
|
||||
with TemporaryWorktree(repo.git_repository) as worktree:
|
||||
assert worktree.temp_directory.startswith(settings.TMP_PATH)
|
||||
|
|
@ -35,13 +34,17 @@ def test_temporary_worktree():
|
|||
|
||||
|
||||
def test_git_repo_init():
|
||||
# This actually works completely without any add-on object and only
|
||||
# creates the necessary file structure
|
||||
repo = AddonGitRepository(1)
|
||||
|
||||
assert repo.git_repository_path == os.path.join(
|
||||
settings.GIT_FILE_STORAGE_PATH, str(1), 'package')
|
||||
assert os.listdir(repo.git_repository_path) == ['.git']
|
||||
|
||||
assert not os.path.exists(repo.git_repository_path)
|
||||
|
||||
# accessing repo.git_repository creates the directory
|
||||
assert sorted(os.listdir(repo.git_repository.path)) == sorted([
|
||||
'objects', 'refs', 'hooks', 'info', 'description', 'config',
|
||||
'HEAD', 'logs'])
|
||||
|
||||
|
||||
def test_git_repo_init_opens_existing_repo():
|
||||
|
|
@ -50,10 +53,14 @@ def test_git_repo_init_opens_existing_repo():
|
|||
|
||||
assert not os.path.exists(expected_path)
|
||||
repo = AddonGitRepository(1)
|
||||
assert not os.path.exists(expected_path)
|
||||
|
||||
# accessing repo.git_repository creates the directory
|
||||
repo.git_repository
|
||||
assert os.path.exists(expected_path)
|
||||
|
||||
repo2 = AddonGitRepository(1)
|
||||
assert repo.git_repository_path == repo2.git_repository_path
|
||||
assert repo.git_repository.path == repo2.git_repository.path
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
|
|
@ -70,7 +77,7 @@ def test_extract_and_commit_from_file_obj():
|
|||
|
||||
# Verify via subprocess to make sure the repositories are properly
|
||||
# read by the regular git client
|
||||
env = {'GIT_DIR': os.path.join(repo.git_repository_path, '.git')}
|
||||
env = {'GIT_DIR': repo.git_repository.path}
|
||||
|
||||
output = subprocess.check_output('git branch', shell=True, env=env)
|
||||
assert 'listed' in output
|
||||
|
|
@ -78,8 +85,8 @@ def test_extract_and_commit_from_file_obj():
|
|||
|
||||
# Test that a new "unlisted" branch is created only if needed
|
||||
repo = AddonGitRepository.extract_and_commit_from_file_obj(
|
||||
addon.current_version.all_files[0],
|
||||
amo.RELEASE_CHANNEL_UNLISTED)
|
||||
file_obj=addon.current_version.all_files[0],
|
||||
channel=amo.RELEASE_CHANNEL_UNLISTED)
|
||||
output = subprocess.check_output('git branch', shell=True, env=env)
|
||||
assert 'listed' in output
|
||||
assert 'unlisted' in output
|
||||
|
|
@ -98,8 +105,8 @@ def test_extract_and_commit_from_file_obj_set_git_hash():
|
|||
assert addon.current_version.git_hash == ''
|
||||
|
||||
AddonGitRepository.extract_and_commit_from_file_obj(
|
||||
addon.current_version.all_files[0],
|
||||
amo.RELEASE_CHANNEL_LISTED)
|
||||
file_obj=addon.current_version.all_files[0],
|
||||
channel=amo.RELEASE_CHANNEL_LISTED)
|
||||
|
||||
addon.current_version.refresh_from_db()
|
||||
assert len(addon.current_version.git_hash) == 40
|
||||
|
|
@ -121,7 +128,7 @@ def test_extract_and_commit_from_file_obj_multiple_versions():
|
|||
|
||||
# Verify via subprocess to make sure the repositories are properly
|
||||
# read by the regular git client
|
||||
env = {'GIT_DIR': os.path.join(repo.git_repository_path, '.git')}
|
||||
env = {'GIT_DIR': repo.git_repository.path}
|
||||
|
||||
output = subprocess.check_output('git branch', shell=True, env=env)
|
||||
assert 'listed' in output
|
||||
|
|
@ -137,15 +144,15 @@ def test_extract_and_commit_from_file_obj_multiple_versions():
|
|||
addon=addon, file_kw={'filename': 'webextension_no_id.xpi'},
|
||||
version='0.2')
|
||||
AddonGitRepository.extract_and_commit_from_file_obj(
|
||||
version.all_files[0],
|
||||
amo.RELEASE_CHANNEL_LISTED)
|
||||
file_obj=version.all_files[0],
|
||||
channel=amo.RELEASE_CHANNEL_LISTED)
|
||||
|
||||
version = version_factory(
|
||||
addon=addon, file_kw={'filename': 'webextension_no_id.xpi'},
|
||||
version='0.3')
|
||||
repo = AddonGitRepository.extract_and_commit_from_file_obj(
|
||||
version.all_files[0],
|
||||
amo.RELEASE_CHANNEL_LISTED)
|
||||
file_obj=version.all_files[0],
|
||||
channel=amo.RELEASE_CHANNEL_LISTED)
|
||||
|
||||
output = subprocess.check_output('git log listed', shell=True, env=env)
|
||||
assert output.count('Create new version') == 3
|
||||
|
|
@ -160,3 +167,46 @@ def test_extract_and_commit_from_file_obj_multiple_versions():
|
|||
output = subprocess.check_output('git log', shell=True, env=env)
|
||||
assert output.count('Mozilla Add-ons Robot') == 1
|
||||
assert '0.1' not in output
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_extract_and_commit_from_file_obj_use_applied_author():
|
||||
addon = addon_factory(file_kw={'filename': 'webextension_no_id.xpi'})
|
||||
user = user_factory(
|
||||
email='fancyuser@foo.bar', display_name='Fancy Test User')
|
||||
|
||||
repo = AddonGitRepository.extract_and_commit_from_file_obj(
|
||||
file_obj=addon.current_version.all_files[0],
|
||||
channel=amo.RELEASE_CHANNEL_LISTED,
|
||||
author=user)
|
||||
|
||||
env = {'GIT_DIR': repo.git_repository.path}
|
||||
|
||||
output = subprocess.check_output(
|
||||
'git log --format=full listed', shell=True, env=env)
|
||||
assert 'Author: Fancy Test User <fancyuser@foo.bar>' in output
|
||||
assert (
|
||||
'Commit: Mozilla Add-ons Robot '
|
||||
'<addons-dev-automation+github@mozilla.com>'
|
||||
in output)
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_extract_and_commit_from_file_obj_use_addons_robot_default():
|
||||
addon = addon_factory(file_kw={'filename': 'webextension_no_id.xpi'})
|
||||
repo = AddonGitRepository.extract_and_commit_from_file_obj(
|
||||
file_obj=addon.current_version.all_files[0],
|
||||
channel=amo.RELEASE_CHANNEL_LISTED)
|
||||
|
||||
env = {'GIT_DIR': repo.git_repository.path}
|
||||
|
||||
output = subprocess.check_output(
|
||||
'git log --format=full listed', shell=True, env=env)
|
||||
assert (
|
||||
'Author: Mozilla Add-ons Robot '
|
||||
'<addons-dev-automation+github@mozilla.com>'
|
||||
in output)
|
||||
assert (
|
||||
'Commit: Mozilla Add-ons Robot '
|
||||
'<addons-dev-automation+github@mozilla.com>'
|
||||
in output)
|
||||
|
|
|
|||
|
|
@ -11,6 +11,7 @@ from django.utils.functional import cached_property
|
|||
from django.utils.translation import ugettext
|
||||
|
||||
import jinja2
|
||||
import waffle
|
||||
|
||||
from django_extensions.db.fields.json import JSONField
|
||||
from django_statsd.clients import statsd
|
||||
|
|
@ -33,6 +34,7 @@ from olympia.files import utils
|
|||
from olympia.files.models import File, cleanup_file
|
||||
from olympia.translations.fields import (
|
||||
LinkifiedField, PurifiedField, TranslatedField, save_signal)
|
||||
from olympia.lib.git import AddonGitRepository
|
||||
|
||||
from .compare import version_dict, version_int
|
||||
|
||||
|
|
@ -247,6 +249,13 @@ class Version(OnChangeMixin, ModelBase):
|
|||
storage.delete(upload.path)
|
||||
version_uploaded.send(sender=version)
|
||||
|
||||
if waffle.switch_is_active('enable-uploads-commit-to-git-storage'):
|
||||
# Extract into git repository
|
||||
AddonGitRepository.extract_and_commit_from_file_obj(
|
||||
file_obj=version.all_files[0],
|
||||
channel=channel,
|
||||
author=upload.user)
|
||||
|
||||
# Generate a preview and icon for listed static themes
|
||||
if (addon.type == amo.ADDON_STATICTHEME and
|
||||
channel == amo.RELEASE_CHANNEL_LISTED):
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ from django.core.files.storage import default_storage as storage
|
|||
|
||||
import mock
|
||||
import pytest
|
||||
from waffle.testutils import override_switch
|
||||
|
||||
from olympia import amo, core
|
||||
from olympia.activity.models import ActivityLog
|
||||
|
|
@ -16,7 +17,8 @@ from olympia.addons.models import (
|
|||
Addon, AddonFeatureCompatibility, AddonReviewerFlags, CompatOverride,
|
||||
CompatOverrideRange)
|
||||
from olympia.amo.templatetags.jinja_helpers import user_media_url
|
||||
from olympia.amo.tests import TestCase, addon_factory, version_factory
|
||||
from olympia.amo.tests import (
|
||||
TestCase, addon_factory, version_factory, user_factory)
|
||||
from olympia.amo.tests.test_models import BasePreviewMixin
|
||||
from olympia.amo.utils import utc_millesecs_from_epoch
|
||||
from olympia.applications.models import AppVersion
|
||||
|
|
@ -29,6 +31,7 @@ from olympia.users.models import UserProfile
|
|||
from olympia.versions.compare import version_int
|
||||
from olympia.versions.models import (
|
||||
ApplicationsVersions, source_upload_path, Version, VersionPreview)
|
||||
from olympia.lib.git import AddonGitRepository
|
||||
|
||||
|
||||
pytestmark = pytest.mark.django_db
|
||||
|
|
@ -869,6 +872,36 @@ class TestExtensionVersionFromUpload(TestVersionFromUpload):
|
|||
amo.RELEASE_CHANNEL_LISTED, parsed_data=self.dummy_parsed_data)
|
||||
assert upload_version.nomination == pending_version.nomination
|
||||
|
||||
@override_switch('enable-uploads-commit-to-git-storage', active=False)
|
||||
def test_doesnt_commit_to_git_by_default(self):
|
||||
addon = addon_factory()
|
||||
upload = self.get_upload('webextension_no_id.xpi')
|
||||
user = user_factory(username='fancyuser')
|
||||
parsed_data = parse_addon(upload, addon, user=user)
|
||||
version = Version.from_upload(
|
||||
upload, addon, [self.selected_app],
|
||||
amo.RELEASE_CHANNEL_LISTED,
|
||||
parsed_data=parsed_data)
|
||||
assert version.pk
|
||||
|
||||
repo = AddonGitRepository(addon.pk)
|
||||
assert not os.path.exists(repo.git_repository_path)
|
||||
|
||||
@override_switch('enable-uploads-commit-to-git-storage', active=True)
|
||||
def test_commits_to_git_waffle_enabled(self):
|
||||
addon = addon_factory()
|
||||
upload = self.get_upload('webextension_no_id.xpi')
|
||||
user = user_factory(username='fancyuser')
|
||||
parsed_data = parse_addon(upload, addon, user=user)
|
||||
version = Version.from_upload(
|
||||
upload, addon, [self.selected_app],
|
||||
amo.RELEASE_CHANNEL_LISTED,
|
||||
parsed_data=parsed_data)
|
||||
assert version.pk
|
||||
|
||||
repo = AddonGitRepository(addon.pk)
|
||||
assert os.path.exists(repo.git_repository_path)
|
||||
|
||||
|
||||
class TestSearchVersionFromUpload(TestVersionFromUpload):
|
||||
filename = 'search.xml'
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче