Merge ~racb/usd-importer:commit-authorship into usd-importer:master

Proposed by Robie Basak
Status: Merged
Merged at revision: 90560ab5cbace79d7a97d46ee447210f1b768c90
Proposed branch: ~racb/usd-importer:commit-authorship
Merge into: usd-importer:master
Diff against target: 1179 lines (+433/-294)
18 files modified
gitubuntu/build.py (+4/-6)
gitubuntu/changelog_tests/maintainer_name_inner_space (+5/-0)
gitubuntu/changelog_tests/maintainer_name_leading_space (+5/-0)
gitubuntu/changelog_tests/maintainer_name_trailing_space (+5/-0)
gitubuntu/changelog_tests/test_maintainer_1 (+5/-0)
gitubuntu/changelog_tests/test_maintainer_2 (+5/-0)
gitubuntu/changelog_tests/test_maintainer_3 (+5/-0)
gitubuntu/git_repository.py (+83/-138)
gitubuntu/git_repository_test.py (+180/-0)
gitubuntu/importer.py (+34/-68)
gitubuntu/importer_tag_test.py (+16/-18)
gitubuntu/importer_test.py (+43/-46)
gitubuntu/importlocal.py (+0/-4)
gitubuntu/queue.py (+4/-7)
gitubuntu/repo_builder.py (+15/-6)
gitubuntu/source_information.py (+4/-0)
gitubuntu/spec.py (+10/-0)
setup.py (+10/-1)
Reviewer Review Type Date Requested Status
Server Team CI bot continuous-integration Approve
Bryce Harrington Approve
Review via email: mp+376593@code.launchpad.net

Commit message

Make Jenkins happy

Description of the change

Unfortunately the final commit of this branch is quite large. I don't see any easy way of breaking it down since it is all a necessary consequence of the method rewrite, and doing it in smaller pieces would break the other pieces. It doesn't seem worth attempting to incrementally transform the old method first.

There isn't a test for the bug being fixed here because we're removing the broken attribute fetch entirely. It doesn't seem worth injecting a fake Launchpad object to ensure that the attribute isn't being fetched, as there's nothing that uses that attribute in the code base any more.

To post a comment you must log in.
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:887e294f7d0bdfd5e6cec9364a1f74dbdbef864f
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/430/
Executed test runs:
    SUCCESS: VM Setup
    SUCCESS: Build
    FAILED: Unit Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/430//rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Bryce Harrington (bryce) wrote :

The test failure is because the patch changes the import_unapplied_dsc() signature to drop the fallback_author and fallback_date parameters, and importlocal.py has not been updated. It looks like you can just drop these lines from the call.

I notice fallback_author also appears in build.py and queue.py (which I guess don't show up as test failures simply due to lack of tests), so those will need updated too.

Given the above, would probably be worthwhile to smoke test the CLI before landing it by manually running the commands to do a dummy merge / build / lint / etc.

The code itself looks good. There's some test cases without pydocs but they're straightforward enough not to need them, with the exception that mention should be made in the test cases where stuff from the changelog_tests/ directory are being used. See inline comments for this and a few miscellany.

review: Needs Fixing
Revision history for this message
Robie Basak (racb) wrote :

Thanks for the review!

I have fixed and smoke tested build-source (since this is based prior to your collapse into build -S, but the same code is used in both cases), import-ppa and queue. import-local fails, but it failed before for an unrelated reason, so my adjustment to import-local is best-effort only. These fixes are in commit 9642080. Further comments inline.

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:9642080cd7779d20ec888fca9cd62cbf18c7b3c0
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/435/
Executed test runs:
    SUCCESS: VM Setup
    SUCCESS: Build
    SUCCESS: Unit Tests
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/435//rebuild

review: Approve (continuous-integration)
Revision history for this message
Bryce Harrington (bryce) wrote :

Looks good.

I've rerun the tests locally (on the eoan lxc container with the StopIteration bug), and looks like the issues have been resolved.

Launchpad appears to have lost the in-line comments (or at least, it's not obvious to me where they've gone), but spot checking the issues I remember flagging before they appear to all be resolved now.

review: Approve
Revision history for this message
Robie Basak (racb) wrote :

I have squashed, rebased on master, resolved one merge conflict and force pushed for a final CI check before merging.

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:90560ab5cbace79d7a97d46ee447210f1b768c90
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/439/
Executed test runs:
    SUCCESS: VM Setup
    SUCCESS: Build
    SUCCESS: Unit Tests
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/439//rebuild

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/gitubuntu/build.py b/gitubuntu/build.py
2index f31a2eb..290427b 100644
3--- a/gitubuntu/build.py
4+++ b/gitubuntu/build.py
5@@ -841,13 +841,11 @@ def do_build(
6
7 # Did we do a quiltify/changelogify fixup?
8 if commitish_tree_hash != tree_hash:
9- fixup_commit_hash = repo.commit_tree_hash(
10- tree_hash=tree_hash,
11- parents=[commitish],
12+ fixup_commit_hash = str(repo.commit_source_tree(
13+ tree=pygit2.Oid(hex=tree_hash),
14+ parents=[pygit2.Oid(hex=commitish)],
15 log_message=b'Automatically generated git-ubuntu fixup.',
16- fallback_author=None,
17- fallback_date=None,
18- )
19+ ))
20 logging.info(
21 """We automatically generated fixup changes relative to
22 %s as commit %s.
23diff --git a/gitubuntu/changelog_tests/maintainer_name_inner_space b/gitubuntu/changelog_tests/maintainer_name_inner_space
24new file mode 100644
25index 0000000..0f0b538
26--- /dev/null
27+++ b/gitubuntu/changelog_tests/maintainer_name_inner_space
28@@ -0,0 +1,5 @@
29+testpkg (1.0) xenial; urgency=medium
30+
31+ * Dummy entry.
32+
33+ -- Test Maintainer <test-maintainer@example.com> Thu, 01 Jan 1970 00:00:00 +0000
34diff --git a/gitubuntu/changelog_tests/maintainer_name_leading_space b/gitubuntu/changelog_tests/maintainer_name_leading_space
35new file mode 100644
36index 0000000..38b507e
37--- /dev/null
38+++ b/gitubuntu/changelog_tests/maintainer_name_leading_space
39@@ -0,0 +1,5 @@
40+testpkg (1.0) xenial; urgency=medium
41+
42+ * Dummy entry.
43+
44+ -- Test Maintainer <test-maintainer@example.com> Thu, 01 Jan 1970 00:00:00 +0000
45diff --git a/gitubuntu/changelog_tests/maintainer_name_trailing_space b/gitubuntu/changelog_tests/maintainer_name_trailing_space
46new file mode 100644
47index 0000000..36a3407
48--- /dev/null
49+++ b/gitubuntu/changelog_tests/maintainer_name_trailing_space
50@@ -0,0 +1,5 @@
51+testpkg (1.0) xenial; urgency=medium
52+
53+ * Dummy entry.
54+
55+ -- Test Maintainer <test-maintainer@example.com> Thu, 01 Jan 1970 00:00:00 +0000
56diff --git a/gitubuntu/changelog_tests/test_maintainer_1 b/gitubuntu/changelog_tests/test_maintainer_1
57new file mode 100644
58index 0000000..abb45c9
59--- /dev/null
60+++ b/gitubuntu/changelog_tests/test_maintainer_1
61@@ -0,0 +1,5 @@
62+testpkg (1.0) xenial; urgency=medium
63+
64+ * Dummy entry.
65+
66+ -- Test Maintainer <test-maintainer@donotmail.com> Thu, 01 Jan 1970 00:00:00 +0000
67diff --git a/gitubuntu/changelog_tests/test_maintainer_2 b/gitubuntu/changelog_tests/test_maintainer_2
68new file mode 100644
69index 0000000..921d4e8
70--- /dev/null
71+++ b/gitubuntu/changelog_tests/test_maintainer_2
72@@ -0,0 +1,5 @@
73+testpkg (1.0) xenial; urgency=medium
74+
75+ * Dummy entry.
76+
77+ -- <test-maintainer@donotmail.com> Thu, 01 Jan 1970 00:00:00 +0000
78diff --git a/gitubuntu/changelog_tests/test_maintainer_3 b/gitubuntu/changelog_tests/test_maintainer_3
79new file mode 100644
80index 0000000..181aa6e
81--- /dev/null
82+++ b/gitubuntu/changelog_tests/test_maintainer_3
83@@ -0,0 +1,5 @@
84+testpkg (1.0) xenial; urgency=medium
85+
86+ * Dummy entry.
87+
88+ -- <test-maintainer@donotmail.com> Thu, 01 Jan 1970 00:00:00 +0000
89diff --git a/gitubuntu/git_repository.py b/gitubuntu/git_repository.py
90index 20c9b03..bce4573 100644
91--- a/gitubuntu/git_repository.py
92+++ b/gitubuntu/git_repository.py
93@@ -4,6 +4,7 @@
94 import collections
95 from contextlib import contextmanager
96 from copy import copy
97+import datetime
98 import enum
99 from functools import lru_cache
100 import itertools
101@@ -28,6 +29,7 @@ from gitubuntu.run import (
102 run_gbp,
103 run_quilt,
104 )
105+import gitubuntu.spec
106 from gitubuntu.test_util import get_test_changelog
107 import gitubuntu.versioning
108 try:
109@@ -574,8 +576,11 @@ class Changelog:
110 'Old (%s) and new (%s) changelog values do not agree' %
111 (self._shell_maintainer, ret)
112 )
113- return ret
114- return self._shell_maintainer
115+ else:
116+ ret = self._shell_maintainer
117+ if not ret:
118+ raise ValueError("Unable to parse maintainer from changelog")
119+ return ret
120
121 @property
122 def _shell_date(self):
123@@ -677,6 +682,33 @@ class Changelog:
124 return ret
125 return self._shell_srcpkg
126
127+ @property
128+ def git_authorship(self):
129+ """Extract last changelog entry's maintainer and timestamp
130+
131+ Parse the first changelog entry's sign-off line into git's commit
132+ authorship metadata model according to the import specification.
133+
134+ :rtype: tuple(str, str, int, int)
135+ :returns: tuple of name, email, time (in seconds since epoch) and
136+ offset from UTC (in minutes)
137+ :raises ValueError: if the changelog sign-off line cannot be parsed
138+ """
139+ m = re.match(r'(?P<name>.*)<(?P<email>.*)>', self.maintainer)
140+ if m is None:
141+ raise ValueError('Cannot get authorship')
142+
143+ time_tuple = time.strptime(self.date, '%a, %d %b %Y %H:%M:%S %z')
144+ epoch_seconds = int(time.mktime(time_tuple))
145+ tz_offset = (time_tuple.tm_gmtoff or 0) // 60 # seconds -> minutes
146+
147+ return (
148+ m.group('name').strip(),
149+ m.group('email'),
150+ epoch_seconds,
151+ tz_offset,
152+ )
153+
154
155 class GitUbuntuChangelogError(Exception):
156 pass
157@@ -1763,102 +1795,6 @@ class GitUbuntuRepository:
158 _, _, pretty_name = tag.name.partition('refs/tags/')
159 return pretty_name
160
161- def get_commit_authorship(
162- self,
163- ref,
164- fallback_author,
165- fallback_date,
166- ):
167- """Extract last debian/changelog entry's maintainer and date
168-
169- :param ref str Reference name to lookup the changelog of
170- :param fallback_author str Name to use as commit author if
171- @ref's debian/changelog fails to parse
172- :param fallback_date str Date to use as commit date if @ref's
173- debian/changelog fails to parse
174-
175- :rtype tuple(str, str, str)
176- :returns Tuple of name, email and date
177- """
178- try:
179- changelog = self.get_changelog_from_treeish(ref)
180- author = changelog.maintainer
181- date = changelog.date
182- except:
183- logging.exception('Cannot get commit authorship for %s' % ref)
184- sys.exit(1)
185-
186- m = re.match(r'(?P<name>.*)<(?P<email>.*)>', author)
187- name = ''
188- email = ''
189- if m is None:
190- logging.error('Cannot get commit authorship for %s' % ref)
191- sys.exit(1)
192-
193- name = m.group('name')
194- email = m.group('email')
195-
196- # fallback to Launchpad's information for the name
197- # cannot do this for the email, as it requires you are logged in
198- if len(name) == 0 and fallback_author:
199- name = fallback_author
200-
201- if len(name) == 0 or len(email) == 0:
202- logging.error('Cannot get commit authorship for %s' % ref)
203- sys.exit(1)
204-
205- try:
206- # check that date is parseable by git-commit
207- time.strptime(date, '%a, %d %b %Y %H:%M:%S %z')
208- except ValueError:
209- try:
210- # date_published can be None
211- if fallback_date:
212- date = fallback_date
213- except:
214- logging.error('No LP source package publishing date '
215- 'available. Unable to proceed.'
216- )
217- sys.exit(1)
218-
219- return (name, email, date)
220-
221- def get_commit_environment(self, ref, fallback_author, fallback_date):
222- """Get a suitable environment hash to force Git commits to match debian/changelog
223-
224- :param ref str Reference name to lookup the changelog of
225- :param fallback_author str Name to use as the commit author if
226- @ref's debian/changelog fails to parse
227- :param fallback_date str Date to use as the commit date if
228- @ref's debian/changelog fails to parse
229-
230- :rtype dict
231- :returns A hash suitable for passing as the environment to
232- subprocess.* containing GIT_* variables that will force a
233- commit to match values from debian/changelog.
234- """
235- author_name, author_email, author_date = self.get_commit_authorship(
236- ref,
237- fallback_author,
238- fallback_date,
239- )
240- # committer date is the published date if present
241- # if it is not, fall back to the authorship date
242- if fallback_date:
243- committer_date = fallback_date
244- else:
245- # XXX: is this a good fallback?
246- committer_date = author_date
247-
248- return {
249- 'GIT_AUTHOR_NAME': author_name,
250- 'GIT_AUTHOR_EMAIL': author_email,
251- 'GIT_AUTHOR_DATE': author_date,
252- 'GIT_COMMITTER_NAME': 'usd-importer',
253- 'GIT_COMMITTER_EMAIL': 'ubuntu-server@lists.ubuntu.com',
254- 'GIT_COMMITTER_DATE': committer_date,
255- }
256-
257 def create_tracking_branch(self, branch_name, upstream_name, force=False):
258 return self.raw_repo.create_branch(
259 branch_name,
260@@ -1930,51 +1866,60 @@ class GitUbuntuRepository:
261 )
262 raise
263
264- def commit_tree_hash(
265+ def commit_source_tree(
266 self,
267- tree_hash,
268+ tree,
269 parents,
270 log_message,
271- fallback_author,
272- fallback_date,
273+ commit_date=None,
274 ):
275- """Commit a Git tree with appropriate parents and message
276-
277- :param tree_hash str Git tree hash
278- :param parents list(str) Git commit hashes of parents
279- :param log_message bytes Commit message
280- :param fallback_author str Git commit author to use if
281- @tree_hash's debian/changelog does not parse
282- :param fallback_date str Git commit date to use if
283- @tree_hash's debian/changelog does not parse
284+ """Commit a git tree with appropriate parents and message
285+
286+ Given a git tree that contains a source package, create a matching
287+ commit using metadata derived from the tree as required according to
288+ the import specification.
289+
290+ Commit metadata elements that are not specified as derived from the
291+ tree itself are required as parameters.
292+
293+ :param pygit2.Oid tree: reference to the git tree in this repository
294+ that contains a debian/changelog file
295+ :param list(pygit2.Oid) parents: parent commits of the commit to be
296+ created
297+ :param bytes log_message: commit message
298+ :param datetime.datetime commit_date: the commit date to use (any
299+ sub-second part of the timestamp is truncated). If None, use the
300+ current date.
301+ :returns: reference to the created commit
302+ :rtype: pygit2.Oid
303 """
304- commit_tree = ['git', 'commit-tree', '--no-gpg-sign', tree_hash]
305-
306- for parent in parents:
307- commit_tree.extend(['-p', parent])
308-
309- commit_env = dict(self.env) # take a copy for modification
310- if fallback_author or fallback_date:
311- commit_env.update(
312- self.get_commit_environment(
313- tree_hash,
314- fallback_author,
315- fallback_date,
316- )
317- )
318- with tempfile.NamedTemporaryFile() as fp:
319- fp.write(log_message)
320- fp.flush()
321- commit_tree += ['-F', fp.name]
322- try:
323- stdout, _ = run(commit_tree, env=commit_env)
324- return stdout.strip()
325- except CalledProcessError:
326- _, t = tempfile.mkstemp()
327- shutil.copy(fp.name, t)
328- logging.exception("Unable to commit tree, saved"
329- "temp commit message as %s", t)
330- sys.exit(1)
331+ if commit_date is None:
332+ commit_date = datetime.datetime.now()
333+
334+ changelog = self.get_changelog_from_treeish(str(tree))
335+
336+ # Divide by 60 for seconds -> minutes
337+ commit_date_offset_td = commit_date.utcoffset()
338+ commit_date_offset_mins = (
339+ int(commit_date_offset_td.total_seconds()) // 60
340+ if commit_date_offset_td
341+ else 0
342+ )
343+
344+ return self.raw_repo.create_commit(
345+ None, # ref: do not update any ref
346+ pygit2.Signature(*changelog.git_authorship), # author
347+ pygit2.Signature( # committer
348+ name=gitubuntu.spec.SYNTHESIZED_COMMITTER_NAME,
349+ email=gitubuntu.spec.SYNTHESIZED_COMMITTER_EMAIL,
350+ time=int(commit_date.timestamp()),
351+ offset=commit_date_offset_mins,
352+ ),
353+ log_message, # message
354+ tree, # tree
355+ parents, # parents
356+ )
357+
358
359 @classmethod
360 def _create_replacement_tree_builder(cls, repo, treeish, sub_path):
361diff --git a/gitubuntu/git_repository_test.py b/gitubuntu/git_repository_test.py
362index 01d3525..5c3f95a 100644
363--- a/gitubuntu/git_repository_test.py
364+++ b/gitubuntu/git_repository_test.py
365@@ -1,6 +1,8 @@
366 import copy
367+import datetime
368 import itertools
369 import os
370+import pkg_resources
371 import shutil
372 import tempfile
373 import unittest
374@@ -20,6 +22,7 @@ from gitubuntu.repo_builder import (
375 Tree,
376 )
377 from gitubuntu.source_builder import Source, SourceSpec
378+import gitubuntu.spec
379 from gitubuntu.test_fixtures import (
380 repo,
381 pygit2_repo,
382@@ -58,6 +61,84 @@ def test_changelog_date():
383 assert test_changelog.date == 'Mon, 12 May 2016 08:14:34 -0700'
384
385
386+@pytest.mark.parametrize('changelog_name, expected', [
387+ ('test_maintainer_1', 'Test Maintainer <test-maintainer@donotmail.com>'),
388+ ('test_maintainer_2', '<test-maintainer@donotmail.com>'),
389+])
390+def test_changelog_maintainer(changelog_name, expected):
391+ test_changelog = get_test_changelog(changelog_name)
392+ assert test_changelog.maintainer == expected
393+
394+
395+def test_changelog_maintainer_invalid():
396+ with pytest.raises(ValueError):
397+ test_changelog = get_test_changelog('test_maintainer_3')
398+ test_changelog.maintainer
399+
400+
401+@pytest.mark.parametrize(
402+ 'changelog_name, name, email, epoch_seconds, offset', [
403+ (
404+ 'test_maintainer_1',
405+ 'Test Maintainer',
406+ 'test-maintainer@donotmail.com',
407+ 0,
408+ 0,
409+ ),
410+ (
411+ 'test_maintainer_2',
412+ '',
413+ 'test-maintainer@donotmail.com',
414+ 0,
415+ 0,
416+ ),
417+ (
418+ 'test_date_1',
419+ 'Test Maintainer',
420+ 'test-maintainer@donotmail.com',
421+ 1463040874,
422+ -420,
423+ ),
424+ (
425+ 'test_date_2',
426+ 'Test Maintainer',
427+ 'test-maintainer@donotmail.com',
428+ 1463040874,
429+ -420,
430+ ),
431+ (
432+ 'maintainer_name_leading_space',
433+ 'Test Maintainer',
434+ 'test-maintainer@example.com',
435+ 0,
436+ 0,
437+ ),
438+ (
439+ 'maintainer_name_trailing_space',
440+ 'Test Maintainer',
441+ 'test-maintainer@example.com',
442+ 0,
443+ 0,
444+ ),
445+ (
446+ 'maintainer_name_inner_space',
447+ 'Test Maintainer',
448+ 'test-maintainer@example.com',
449+ 0,
450+ 0,
451+ ),
452+])
453+def test_changelog_authorship(
454+ changelog_name,
455+ name,
456+ email,
457+ epoch_seconds,
458+ offset,
459+):
460+ result = get_test_changelog(changelog_name).git_authorship
461+ assert result == (name, email, epoch_seconds, offset)
462+
463+
464 def test_changelog_utf8():
465 test_changelog = get_test_changelog('test_utf8_error')
466 assert test_changelog.version == '1.0.3-2'
467@@ -697,3 +778,102 @@ def test_repo_does_not_cleanup():
468 assert os.path.exists(path)
469 finally:
470 shutil.rmtree(path, ignore_errors=True)
471+
472+
473+def pygit2_signature_tuple(sig):
474+ """Convert a pygit2.Signature object to a tuple for equality comparison
475+
476+ Support for comparing pygit2.Signature objects directly was added in pygit2
477+ 0.27.3. Since we are pinned to an earlier version, this function converts a
478+ pygit2.Signature to a tuple that can be compared. When we use pygit2 >=
479+ 0.27.3, this function can be removed and callers changed to compare
480+ pygit2.Signature objects directly.
481+
482+ For interface stability, the returned tuple should be used for equality
483+ comparisons only.
484+
485+ :param pygit2.Signature sig: any Signature object
486+ :returns: the given Signature objected converted to an (opaque) tuple that
487+ can be compared for equality
488+ :rtype: tuple
489+ """
490+ return tuple(getattr(sig, k) for k in ['name', 'email', 'time', 'offset'])
491+
492+
493+def test_commit_tree(repo):
494+ # Construct a repository with an initial commit on the master branch so
495+ # that we can verify later parentage and lack of branch movement
496+ parent_commit_oid = Repo(
497+ commits=[Commit(name='master')],
498+ branches={'master': Placeholder('master')}
499+ ).write(repo.raw_repo)
500+ repo.raw_repo.lookup_reference('HEAD').set_target('refs/heads/master')
501+
502+ # Construct a tree inside the repository with a debian/changelog in it to
503+ # feed to the method under test
504+ test_changelog_path = os.path.join(
505+ pkg_resources.resource_filename('gitubuntu', 'changelog_tests'),
506+ 'test_date_1',
507+ )
508+ with open(test_changelog_path, 'rb') as f:
509+ test_changelog_bytes = f.read()
510+ test_changelog_blob = Blob(test_changelog_bytes)
511+ source_tree = Tree({'debian': Tree({'changelog': test_changelog_blob})})
512+ source_tree_oid = source_tree.write(repo.raw_repo)
513+
514+ # Call the method under test
515+ commit_oid = repo.commit_source_tree(
516+ tree=source_tree_oid,
517+ parents=[parent_commit_oid],
518+ log_message='test_commit_msg',
519+ commit_date=datetime.datetime(
520+ 1971, # year
521+ 2, # month
522+ 3, # day
523+ 4, # hours
524+ 5, # minutes
525+ 6, # seconds
526+ 7, # milliseconds (this should get truncated down to 0)
527+ datetime.timezone(datetime.timedelta(hours=-8))
528+ ),
529+ )
530+
531+ # Retrieve the commit object created
532+ commit = repo.raw_repo[commit_oid]
533+
534+ # Verify the commit object created is exactly as we expect it
535+
536+ # Neither HEAD nor the master branch should have changed
537+ assert repo.raw_repo.lookup_reference('HEAD').target == 'refs/heads/master'
538+ master_ref = repo.raw_repo.lookup_reference('refs/heads/master')
539+ assert master_ref.target == parent_commit_oid
540+
541+ # The tree of the commit should match the tree we requested
542+ assert commit.tree_id == source_tree_oid
543+
544+ # The parent commits should match the parents we requested
545+ assert commit.parent_ids == [parent_commit_oid]
546+
547+ # The commit message should match what we requested
548+ assert commit.message == 'test_commit_msg'
549+
550+ # The commit author should match the changelog entry inside the tree given
551+ assert pygit2_signature_tuple(commit.author) == pygit2_signature_tuple(
552+ pygit2.Signature(
553+ name='Test Maintainer',
554+ email='test-maintainer@donotmail.com',
555+ time=1463040874,
556+ offset=-420,
557+ )
558+ )
559+
560+ # The commit committer should match the date we requested (except the
561+ # milliseconds component) combined with the specification name and email
562+ assert pygit2_signature_tuple(commit.committer) == pygit2_signature_tuple(
563+ pygit2.Signature(
564+ name=gitubuntu.spec.SYNTHESIZED_COMMITTER_NAME,
565+ email=gitubuntu.spec.SYNTHESIZED_COMMITTER_EMAIL,
566+ time=34430706,
567+ offset=-480,
568+ )
569+ )
570diff --git a/gitubuntu/importer.py b/gitubuntu/importer.py
571index f7f78a6..c6bfea9 100644
572--- a/gitubuntu/importer.py
573+++ b/gitubuntu/importer.py
574@@ -578,8 +578,7 @@ def _commit_import(
575 changelog_parent_commit,
576 upload_parent_commit,
577 unapplied_parent_commit,
578- fallback_author,
579- fallback_date,
580+ commit_date,
581 ):
582 """Commit a tree object into the repository with the specified
583 parents
584@@ -602,10 +601,7 @@ def _commit_import(
585 upload parent. Should be None if patches-applied import.
586 :param unapplied_parent_commit str Git commit hash of unapplied
587 parent. Should be None if patches-unapplied import.
588- :param fallback_author str If the authorship for @version cannot be
589- parsed, use this value. Can be None.
590- :param fallback_date str If the date for @version cannot be parsed,
591- use this value. Can be None.
592+ :param str commit_date: committer date to use for the git commit metadata
593
594 :rtype str
595 :returns Hash of created commit
596@@ -619,16 +615,16 @@ def _commit_import(
597 parents = []
598
599 if changelog_parent_commit is not None:
600- parents.append(changelog_parent_commit)
601+ parents.append(pygit2.Oid(hex=changelog_parent_commit))
602 if upload_parent_commit is not None:
603- parents.append(upload_parent_commit)
604+ parents.append(pygit2.Oid(hex=upload_parent_commit))
605 if unapplied_parent_commit is not None:
606- parents.append(unapplied_parent_commit)
607+ parents.append(pygit2.Oid(hex=unapplied_parent_commit))
608
609- commit_hash = repo.commit_tree_hash(
610- tree_hash,
611- parents,
612- get_import_commit_msg(
613+ commit_hash = repo.commit_source_tree(
614+ tree=pygit2.Oid(hex=tree_hash),
615+ parents=parents,
616+ log_message=get_import_commit_msg(
617 repo=repo,
618 version=version,
619 target_head_name=target_head_name,
620@@ -638,11 +634,10 @@ def _commit_import(
621 upload_parent_commit=upload_parent_commit,
622 unapplied_parent_commit=unapplied_parent_commit,
623 ),
624- fallback_author,
625- fallback_date,
626+ commit_date=commit_date,
627 )
628
629- return commit_hash
630+ return str(commit_hash)
631
632 def commit_unapplied_patches_import(
633 repo,
634@@ -652,8 +647,7 @@ def commit_unapplied_patches_import(
635 namespace,
636 changelog_parent_commit,
637 upload_parent_commit,
638- fallback_author,
639- fallback_date,
640+ commit_date,
641 ):
642 return _commit_import(
643 repo,
644@@ -665,8 +659,7 @@ def commit_unapplied_patches_import(
645 upload_parent_commit,
646 # unapplied trees do not have a distinct unapplied parent
647 None,
648- fallback_author,
649- fallback_date,
650+ commit_date,
651 )
652
653 def commit_applied_patches_import(
654@@ -677,8 +670,7 @@ def commit_applied_patches_import(
655 namespace,
656 changelog_parent_commit,
657 unapplied_parent_commit,
658- fallback_author,
659- fallback_date,
660+ commit_date,
661 ):
662 return _commit_import(
663 repo,
664@@ -691,8 +683,7 @@ def commit_applied_patches_import(
665 # can not have them as direct parents
666 None,
667 unapplied_parent_commit,
668- fallback_author,
669- fallback_date,
670+ commit_date,
671 )
672
673 def import_dsc(repo, dsc, namespace, version, dist):
674@@ -1369,8 +1360,7 @@ def import_unapplied_dsc(
675 head_name,
676 skip_orig,
677 parent_overrides,
678- fallback_author,
679- fallback_date,
680+ commit_date=None,
681 ):
682 """Imports a DSC into a Git repository without patches applied
683
684@@ -1387,10 +1377,8 @@ def import_unapplied_dsc(
685 tarballs. Useful for debug import runs to make them as fast as
686 possible, or to workaround pristine-tar issues.
687 :param parent_overrides dict See parse_parentfile.
688- :param fallback_author str If the authorship for @version cannot be
689- parsed, use this value. Can be None.
690- :param fallback_date str If the date for @version cannot be parsed,
691- use this value. Can be None.
692+ :param str commit_date: committer date to use for the git commit metadata.
693+ If None, use the current date.
694
695 :rtype None
696 """
697@@ -1572,8 +1560,7 @@ def import_unapplied_dsc(
698 namespace,
699 unapplied_changelog_parent_commit,
700 upload_parent_commit,
701- fallback_author,
702- fallback_date,
703+ commit_date,
704 )
705 logging.debug(
706 "Committed patches-unapplied import of %s as %s in %s",
707@@ -1642,13 +1629,6 @@ def import_unapplied_spi(
708
709 repo.clean_repository_state()
710
711- fallback_author = None
712- if spi.spphr.package_creator.display_name:
713- fallback_author = str(spi.spphr.package_creator.display_name)
714- fallback_date = None
715- if spi.date_published:
716- fallback_date = str(spi.date_published)
717-
718 import_unapplied_dsc(
719 repo=repo,
720 version=str(spi.version),
721@@ -1658,8 +1638,7 @@ def import_unapplied_spi(
722 head_name=spi.head_name(namespace),
723 skip_orig=skip_orig,
724 parent_overrides=parent_overrides,
725- fallback_author=fallback_author,
726- fallback_date=fallback_date,
727+ commit_date=spi.date_created,
728 )
729
730 def import_applied_dsc(
731@@ -1671,8 +1650,7 @@ def import_applied_dsc(
732 head_name,
733 allow_applied_failures,
734 parent_overrides,
735- fallback_author,
736- fallback_date,
737+ commit_date=None,
738 ):
739 """Imports a DSC into a Git repository with patches applied
740
741@@ -1688,10 +1666,8 @@ def import_applied_dsc(
742 :param allow_applied_failures bool If True, failure to apply quilt
743 patches is not fatal.
744 :param parent_overrides dict See parse_parentfile.
745- :param fallback_author str If the authorship for @version cannot be
746- parsed, use this value. Can be None.
747- :param fallback_date str If the date for @version cannot be parsed,
748- use this value. Can be None.
749+ :param str commit_date: committer date to use for the git commit metadata.
750+ If None, use the current date.
751
752 :rtype None
753 """
754@@ -1699,7 +1675,7 @@ def import_applied_dsc(
755 _, _, pretty_head_name = head_name.partition('%s/' % namespace)
756
757 unapplied_parent_tag = repo.get_import_tag(version, namespace)
758- unapplied_parent_commit = str(unapplied_parent_tag.peel().id)
759+ unapplied_parent_commit = unapplied_parent_tag.peel().id
760 unapplied_import_tree_hash = str(unapplied_parent_tag.peel(pygit2.Tree).id)
761
762 # this is the result of all patches being applied
763@@ -1712,7 +1688,7 @@ def import_applied_dsc(
764 logging.debug(
765 "Found patches-unapplied version %s as commit %s",
766 version,
767- unapplied_parent_commit,
768+ str(unapplied_parent_commit),
769 )
770
771 import_tree_versions = repo.get_all_changelog_versions_from_treeish(
772@@ -1812,17 +1788,16 @@ def import_applied_dsc(
773 patch_name.encode()
774 )
775
776- unapplied_parent_commit = repo.commit_tree_hash(
777- applied_import_tree_hash,
778- [unapplied_parent_commit],
779- msg,
780- fallback_author,
781- fallback_date,
782+ unapplied_parent_commit = repo.commit_source_tree(
783+ tree=pygit2.Oid(hex=applied_import_tree_hash),
784+ parents=[unapplied_parent_commit],
785+ log_message=msg,
786+ commit_date=commit_date,
787 )
788
789 logging.debug(
790 "Committed patch-application of %s as %s",
791- patch_name, unapplied_parent_commit
792+ patch_name, str(unapplied_parent_commit)
793 )
794 except CalledProcessError as e:
795 if allow_applied_failures:
796@@ -1836,9 +1811,8 @@ def import_applied_dsc(
797 applied_import_tree_hash,
798 namespace,
799 applied_changelog_parent_commit,
800- unapplied_parent_commit,
801- fallback_author,
802- fallback_date,
803+ str(unapplied_parent_commit),
804+ commit_date,
805 )
806 logging.debug(
807 "Committed patches-applied import of %s as %s in %s",
808@@ -1902,13 +1876,6 @@ def import_applied_spi(
809
810 repo.clean_repository_state()
811
812- fallback_author = None
813- if spi.spphr.package_creator.display_name:
814- fallback_author = str(spi.spphr.package_creator.display_name)
815- fallback_date = None
816- if spi.date_published:
817- fallback_date = str(spi.date_published)
818-
819 import_applied_dsc(
820 repo=repo,
821 version=str(spi.version),
822@@ -1918,8 +1885,7 @@ def import_applied_spi(
823 head_name=spi.applied_head_name(namespace),
824 allow_applied_failures=allow_applied_failures,
825 parent_overrides=parent_overrides,
826- fallback_author=fallback_author,
827- fallback_date=fallback_date,
828+ commit_date=spi.date_created,
829 )
830
831
832diff --git a/gitubuntu/importer_tag_test.py b/gitubuntu/importer_tag_test.py
833index 901ab10..c2ae1e2 100644
834--- a/gitubuntu/importer_tag_test.py
835+++ b/gitubuntu/importer_tag_test.py
836@@ -461,15 +461,14 @@ def test_import_unapplied_spi_tags(
837 native=False,
838 )
839
840- with patch_get_commit_environment(repo):
841- with source_builder.Source(publish_spec) as dsc_path:
842- target.import_unapplied_spi(
843- repo=repo,
844- spi=MockSPI(dsc_path, publish_spec.version),
845- namespace='importer',
846- skip_orig=False,
847- parent_overrides={},
848- )
849+ with source_builder.Source(publish_spec) as dsc_path:
850+ target.import_unapplied_spi(
851+ repo=repo,
852+ spi=MockSPI(dsc_path, publish_spec.version),
853+ namespace='importer',
854+ skip_orig=False,
855+ parent_overrides={},
856+ )
857
858 if reuse:
859 # Test that the previous commit was reused. In this case, the
860@@ -711,15 +710,14 @@ def test_import_applied_spi_tags(
861 has_patches=True,
862 )
863
864- with patch_get_commit_environment(repo):
865- with source_builder.Source(publish_spec) as dsc_path:
866- target.import_applied_spi(
867- repo=repo,
868- spi=MockSPI(dsc_path, publish_spec.version),
869- namespace='importer',
870- allow_applied_failures=False,
871- parent_overrides={},
872- )
873+ with source_builder.Source(publish_spec) as dsc_path:
874+ target.import_applied_spi(
875+ repo=repo,
876+ spi=MockSPI(dsc_path, publish_spec.version),
877+ namespace='importer',
878+ allow_applied_failures=False,
879+ parent_overrides={},
880+ )
881
882 if reuse:
883 # Test that the previous commit was reused. In this case,
884diff --git a/gitubuntu/importer_test.py b/gitubuntu/importer_test.py
885index 0ce014a..90412d4 100644
886--- a/gitubuntu/importer_test.py
887+++ b/gitubuntu/importer_test.py
888@@ -4,6 +4,7 @@ from unittest.mock import (
889 sentinel,
890 )
891
892+import datetime
893 import os
894 import pytest
895 import shutil
896@@ -617,7 +618,7 @@ def MockSPI(dsc_path, version):
897 spi.dsc_pathname = dsc_path
898 spi.distribution_name = 'Ubuntu'
899 spi.version = version
900- spi.date_published = '1970-01-01T00:00:00Z'
901+ spi.date_created = datetime.datetime(1970, 1, 1)
902 head_name = Mock(name='head_name')
903 head_name.return_value = 'importer/ubuntu/trusty'
904 spi.head_name = head_name
905@@ -665,18 +666,17 @@ def test_import_unapplied_spi_quilt_patches(
906 branches={'importer/ubuntu/trusty': Placeholder('publish')},
907 )
908
909- with patch_get_commit_environment(repo):
910- with source_builder.Source(publish_spec) as dsc_path:
911- # import_unapplied_spi currently assumes it is called from the
912- # repository directory (pristine-tar and other commands rely on
913- # this)
914- target.import_unapplied_spi(
915- repo=repo,
916- spi=MockSPI(dsc_path, publish_spec.version),
917- namespace='importer',
918- skip_orig=False,
919- parent_overrides={},
920- )
921+ with source_builder.Source(publish_spec) as dsc_path:
922+ # import_unapplied_spi currently assumes it is called from the
923+ # repository directory (pristine-tar and other commands rely on
924+ # this)
925+ target.import_unapplied_spi(
926+ repo=repo,
927+ spi=MockSPI(dsc_path, publish_spec.version),
928+ namespace='importer',
929+ skip_orig=False,
930+ parent_overrides={},
931+ )
932
933 assert repo_comparator.equals(
934 repoA=repo.raw_repo,
935@@ -823,18 +823,17 @@ def test_import_unapplied_spi_parenting(
936 changelog_versions=changelog_versions,
937 )
938
939- with patch_get_commit_environment(repo):
940- with source_builder.Source(publish_spec) as dsc_path:
941- # import_unapplied_spi currently assumes it is called from the
942- # repository directory (pristine-tar and other commands rely on
943- # this)
944- target.import_unapplied_spi(
945- repo=repo,
946- spi=MockSPI(dsc_path, publish_spec.version),
947- namespace='importer',
948- skip_orig=False,
949- parent_overrides={},
950- )
951+ with source_builder.Source(publish_spec) as dsc_path:
952+ # import_unapplied_spi currently assumes it is called from the
953+ # repository directory (pristine-tar and other commands rely on
954+ # this)
955+ target.import_unapplied_spi(
956+ repo=repo,
957+ spi=MockSPI(dsc_path, publish_spec.version),
958+ namespace='importer',
959+ skip_orig=False,
960+ parent_overrides={},
961+ )
962
963 # we would like to check the commit hashes, but we cannot
964 # currently, as the commit messages are specific to the importer
965@@ -887,18 +886,17 @@ def test_import_unapplied_spi_parent_override(
966 changelog_versions=['2-1', '1-1'],
967 )
968
969- with patch_get_commit_environment(repo):
970- with source_builder.Source(publish_spec) as dsc_path:
971- # import_unapplied_spi currently assumes it is called from the
972- # repository directory (pristine-tar and other commands rely on
973- # this)
974- target.import_unapplied_spi(
975- repo=repo,
976- spi=MockSPI(dsc_path, publish_spec.version),
977- namespace='importer',
978- skip_orig=False,
979- parent_overrides={'2-1': {'changelog_parent': '1-2'}},
980- )
981+ with source_builder.Source(publish_spec) as dsc_path:
982+ # import_unapplied_spi currently assumes it is called from the
983+ # repository directory (pristine-tar and other commands rely on
984+ # this)
985+ target.import_unapplied_spi(
986+ repo=repo,
987+ spi=MockSPI(dsc_path, publish_spec.version),
988+ namespace='importer',
989+ skip_orig=False,
990+ parent_overrides={'2-1': {'changelog_parent': '1-2'}},
991+ )
992
993 validation_repo = input_repo.copy(
994 add_commits=[
995@@ -1143,15 +1141,14 @@ def test_import_applied_spi_parenting(
996 has_patches=True,
997 )
998
999- with patch_get_commit_environment(repo):
1000- with source_builder.Source(publish_spec) as dsc_path:
1001- target.import_applied_spi(
1002- repo=repo,
1003- spi=MockSPI(dsc_path, publish_spec.version),
1004- namespace='importer',
1005- allow_applied_failures=False,
1006- parent_overrides={},
1007- )
1008+ with source_builder.Source(publish_spec) as dsc_path:
1009+ target.import_applied_spi(
1010+ repo=repo,
1011+ spi=MockSPI(dsc_path, publish_spec.version),
1012+ namespace='importer',
1013+ allow_applied_failures=False,
1014+ parent_overrides={},
1015+ )
1016
1017 applied_tag = repo.raw_repo.lookup_reference(
1018 'refs/tags/importer/applied/1-2',
1019diff --git a/gitubuntu/importlocal.py b/gitubuntu/importlocal.py
1020index 778a673..f886ca2 100644
1021--- a/gitubuntu/importlocal.py
1022+++ b/gitubuntu/importlocal.py
1023@@ -101,8 +101,6 @@ def main(
1024 head_name=None,
1025 skip_orig=skip_orig,
1026 parent_overrides=parent_overrides,
1027- fallback_author=None,
1028- fallback_date=None,
1029 )
1030
1031 import_applied_dsc(
1032@@ -114,8 +112,6 @@ def main(
1033 head_name=None,
1034 allow_applied_failures=False,
1035 parent_overrides=parent_overrides,
1036- fallback_author=None,
1037- fallback_date=None,
1038 )
1039
1040 repo.clean_repository_state()
1041diff --git a/gitubuntu/queue.py b/gitubuntu/queue.py
1042index 19f61fb..c0fc165 100644
1043--- a/gitubuntu/queue.py
1044+++ b/gitubuntu/queue.py
1045@@ -201,15 +201,12 @@ def _commit_upload(repo, upload, parent_commit):
1046 )
1047
1048 # Ensure parents contains a string if specified
1049- parents = [str(parent_commit)] if parent_commit else []
1050- commit_hash = repo.commit_tree_hash(
1051- tree_hash=tree_hash,
1052+ parents = [pygit2.Oid(hex=str(parent_commit))] if parent_commit else []
1053+ commit_hash = str(repo.commit_source_tree(
1054+ tree=tree_hash,
1055 parents=parents,
1056 log_message='Queue import'.encode(),
1057- fallback_author=None,
1058- fallback_date=None,
1059-
1060- )
1061+ ))
1062
1063 return commit_hash
1064
1065diff --git a/gitubuntu/repo_builder.py b/gitubuntu/repo_builder.py
1066index c779c71..6c3aed3 100644
1067--- a/gitubuntu/repo_builder.py
1068+++ b/gitubuntu/repo_builder.py
1069@@ -5,6 +5,7 @@ import pygit2
1070
1071 import gitubuntu.importer
1072 from gitubuntu.source_builder import Source, SourceSpec
1073+import gitubuntu.spec
1074
1075
1076 """Build test git repositories as data structures
1077@@ -56,13 +57,21 @@ __all__ = [
1078 # hashes or something?
1079
1080
1081-DEFAULT_SIGNATURE = pygit2.Signature(
1082- name='Test Builder',
1083- email='test@example.com',
1084+DEFAULT_AUTHOR_SIGNATURE = pygit2.Signature(
1085+ # Match the default from source_builder.CHANGELOG_TEMPLATE
1086+ name='USD Import Team',
1087+ email='usd-import-team@lists.launchpad.net',
1088 time=0, # don't default to current time for hash reproducibility
1089 offset=0,
1090 )
1091
1092+DEFAULT_COMMITTER_SIGNATURE = pygit2.Signature(
1093+ name=gitubuntu.spec.SYNTHESIZED_COMMITTER_NAME,
1094+ email=gitubuntu.spec.SYNTHESIZED_COMMITTER_EMAIL,
1095+ time=0,
1096+ offset=0,
1097+)
1098+
1099
1100 class NamedMixin:
1101 def __init__(self, name=None):
1102@@ -267,8 +276,8 @@ class Commit(NamedMixin, WriteMixin):
1103 def _obj_to_oid(self, repo, record=None):
1104 return repo.create_commit(
1105 None,
1106- DEFAULT_SIGNATURE,
1107- DEFAULT_SIGNATURE,
1108+ DEFAULT_AUTHOR_SIGNATURE,
1109+ DEFAULT_COMMITTER_SIGNATURE,
1110 self.message,
1111 self.tree.write(repo),
1112 [parent.write(repo, record=record) for parent in self.parents],
1113@@ -334,7 +343,7 @@ class Repo:
1114 name,
1115 target.write(repo),
1116 pygit2.GIT_OBJ_COMMIT,
1117- DEFAULT_SIGNATURE,
1118+ DEFAULT_COMMITTER_SIGNATURE,
1119 'Tag message',
1120 )
1121 return written_commits[0] if written_commits else None
1122diff --git a/gitubuntu/source_information.py b/gitubuntu/source_information.py
1123index 6bd6d9f..38df33e 100644
1124--- a/gitubuntu/source_information.py
1125+++ b/gitubuntu/source_information.py
1126@@ -276,6 +276,10 @@ class GitUbuntuSourcePackageInformation:
1127 return self._spphr.date_published
1128
1129 @property
1130+ def date_created(self):
1131+ return self._spphr.date_created
1132+
1133+ @property
1134 def pocket(self):
1135 return self._spphr.pocket
1136
1137diff --git a/gitubuntu/spec.py b/gitubuntu/spec.py
1138new file mode 100644
1139index 0000000..305829d
1140--- /dev/null
1141+++ b/gitubuntu/spec.py
1142@@ -0,0 +1,10 @@
1143+# This file contains constant strings that form part of the importer
1144+# specification. Changing these strings will mutate commit hashes so must not
1145+# be changed without careful consideration and declaration of a hash ABI break.
1146+
1147+# Changing any importer code or code called from the importer may also cause an
1148+# ABI break of course, but changing these constants certainly will and so are
1149+# placed here to try to help avoid accidents.
1150+
1151+SYNTHESIZED_COMMITTER_NAME = 'usd-importer'
1152+SYNTHESIZED_COMMITTER_EMAIL = 'ubuntu-server@lists.ubuntu.com'
1153diff --git a/setup.py b/setup.py
1154index 7b6b1a0..d9600f9 100644
1155--- a/setup.py
1156+++ b/setup.py
1157@@ -13,12 +13,21 @@ setup(name='gitubuntu',
1158 url="https://code.launchpad.net/~usd-import-team/usd-importer/+git/usd-importer",
1159 maintainer="Ubuntu Server Team",
1160 maintainer_email="usd-import-team@lists.launchpad.net ",
1161+
1162+ # Pinning notes
1163+ #
1164+ # See LP: #1855725 on unnecessary pins.
1165+ #
1166+ # pygit2: when >= 0.27.3, git_repository_test.py::pygit2_signature_tuple
1167+ # can be removed and callers adjusted. See the docstring in that
1168+ # function for details.
1169+
1170 install_requires=[
1171 'argcomplete==1.8.1',
1172 'astroid<2.3', # https://github.com/PyCQA/pylint/issues/3137
1173 'cachetools',
1174 'python-debian==0.1.31',
1175- 'pygit2==0.24.2',
1176+ 'pygit2==0.24.2', # see pinning notes above
1177 'launchpadlib==1.10.6',
1178 'petname',
1179 'pylint<2.4', # https://github.com/PyCQA/pylint/issues/3137#issuecomment-542286971

Subscribers

People subscribed via source and target branches