Page MenuHomeSoftware Heritage
Differential D6000

git_bare: Add support for annotated tags pointing to commits
ClosedPublic
Actions

Authored by vlorentz on Jul 16 2021, 2:26 PM.

Details

Reviewers
ardumont
Group Reviewers
Reviewers
Maniphest Tasks
T843: Vault: Add a "git bare" tarball cooker
Commits
rDVAU73b2b13b9e06: git_bare: Add support for annotated tags pointing to commits
Summary

Depends on D5999.

Diff Detail

Unit TestsFailed
View All

TimeTest
303 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.vault.tests.test_cookers.TestSnapshotCooker::test_snapshot_tags[cook_extract_snapshot_git_bare]
self = <swh.vault.tests.test_cookers.TestSnapshotCooker object at 0x7fda2ff7a6a0> git_loader = <function git_loader.<locals>._create_loader at 0x7fda2ffac268> cook_extract_snapshot = <function cook_extract_snapshot_git_bare at 0x7fda35a99840>
81 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.vault.tests.test_backend::test_available
122 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.vault.tests.test_backend::test_cache_expire_oldest
115 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.vault.tests.test_backend::test_cache_expire_until
96 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.vault.tests.test_backend::test_cook_email_pending_done
View Full Test Results (1 Failed · 93 Passed)

Event Timeline

vlorentz created this revision.Jul 16 2021, 2:26 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptJul 16 2021, 2:26 PM
vlorentz updated this revision to Diff 21660.Jul 16 2021, 2:27 PM

remove comment that shouldn't be committed yet

swh-public-ci added a comment.Jul 16 2021, 2:28 PM

Build has FAILED

Patch application report for D6000 (id=21659)

Could not rebase; Attempt merge onto 5951a5c63b...

Updating 5951a5c..6629eda
Fast-forward
 swh/vault/cookers/__init__.py   |   1 +
 swh/vault/cookers/git_bare.py   |  97 ++++++++++-
 swh/vault/tests/test_cookers.py | 378 ++++++++++++++++++++++++++++++++++------
 3 files changed, 416 insertions(+), 60 deletions(-)
Changes applied before test
commit 6629edaf1e5d5c8c4dc3ca8e668d768373077e84
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Jul 16 14:25:35 2021 +0200

    git_bare: Add support for annotated tags pointing to commits

commit 4cb80723d8fbd094bd5b6439d71cfc2e2e091a86
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Jul 15 19:46:54 2021 +0200

    git_bare: Add partial support for snapshots (no release or swh-graph support yet)

commit 70fc50f40b8bc13b022f96be0027e89175773fd7
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Jul 15 17:20:44 2021 +0200

    tests: Split RepoFixtures off TestRevisionCooker
    
    So the 'load' and 'check' parts of the tests can be reused by future
    snapshot tests.

commit a1f42284c683ec1d0ee169cda67f507529822473
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Jul 15 16:58:55 2021 +0200

    tests: Make TestRevisionCooker more modular
    
    So its 'load' and 'check' parts can be reused for testing snapshots

Link to build: https://jenkins.softwareheritage.org/job/DVAU/job/tests-on-diff/136/
See console output for more information: https://jenkins.softwareheritage.org/job/DVAU/job/tests-on-diff/136/console

Harbormaster failed remote builds in B22629: Diff 21659!Jul 16 2021, 2:28 PM
swh-public-ci added a comment.Jul 16 2021, 2:30 PM

Build has FAILED

Patch application report for D6000 (id=21660)

Rebasing onto 5951a5c63b...

Current branch diff-target is up to date.
Changes applied before test
commit c427dd8017062e37dfe0bfdd3113cd0999e30d55
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Jul 16 14:25:35 2021 +0200

    git_bare: Add support for annotated tags pointing to commits

commit 4cb80723d8fbd094bd5b6439d71cfc2e2e091a86
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Jul 15 19:46:54 2021 +0200

    git_bare: Add partial support for snapshots (no release or swh-graph support yet)

commit 70fc50f40b8bc13b022f96be0027e89175773fd7
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Jul 15 17:20:44 2021 +0200

    tests: Split RepoFixtures off TestRevisionCooker
    
    So the 'load' and 'check' parts of the tests can be reused by future
    snapshot tests.

commit a1f42284c683ec1d0ee169cda67f507529822473
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Jul 15 16:58:55 2021 +0200

    tests: Make TestRevisionCooker more modular
    
    So its 'load' and 'check' parts can be reused for testing snapshots

Link to build: https://jenkins.softwareheritage.org/job/DVAU/job/tests-on-diff/137/
See console output for more information: https://jenkins.softwareheritage.org/job/DVAU/job/tests-on-diff/137/console

Harbormaster returned this revision to the author for changes because remote builds failed.Jul 16 2021, 2:30 PM
Harbormaster failed remote builds in B22630: Diff 21660!
vlorentz updated this revision to Diff 21662.Jul 16 2021, 2:30 PM

change base commit

swh-public-ci added a comment.Jul 16 2021, 2:34 PM

Build has FAILED

Patch application report for D6000 (id=21662)

Could not rebase; Attempt merge onto 5951a5c63b...

Updating 5951a5c..c427dd8
Fast-forward
 swh/vault/cookers/__init__.py   |   1 +
 swh/vault/cookers/git_bare.py   |  94 +++++++++-
 swh/vault/tests/test_cookers.py | 378 ++++++++++++++++++++++++++++++++++------
 3 files changed, 413 insertions(+), 60 deletions(-)
Changes applied before test
commit c427dd8017062e37dfe0bfdd3113cd0999e30d55
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Jul 16 14:25:35 2021 +0200

    git_bare: Add support for annotated tags pointing to commits

commit 4cb80723d8fbd094bd5b6439d71cfc2e2e091a86
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Jul 15 19:46:54 2021 +0200

    git_bare: Add partial support for snapshots (no release or swh-graph support yet)

commit 70fc50f40b8bc13b022f96be0027e89175773fd7
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Jul 15 17:20:44 2021 +0200

    tests: Split RepoFixtures off TestRevisionCooker
    
    So the 'load' and 'check' parts of the tests can be reused by future
    snapshot tests.

commit a1f42284c683ec1d0ee169cda67f507529822473
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Jul 15 16:58:55 2021 +0200

    tests: Make TestRevisionCooker more modular
    
    So its 'load' and 'check' parts can be reused for testing snapshots

Link to build: https://jenkins.softwareheritage.org/job/DVAU/job/tests-on-diff/139/
See console output for more information: https://jenkins.softwareheritage.org/job/DVAU/job/tests-on-diff/139/console

Harbormaster returned this revision to the author for changes because remote builds failed.Jul 16 2021, 2:34 PM
Harbormaster failed remote builds in B22632: Diff 21662!
vlorentz updated this revision to Diff 21663.Jul 16 2021, 2:48 PM

fix test on jenkins

swh-public-ci added a comment.Jul 16 2021, 2:50 PM

Build has FAILED

Patch application report for D6000 (id=21663)

Could not rebase; Attempt merge onto 5951a5c63b...

Updating 5951a5c..6c5f074
Fast-forward
 swh/vault/cookers/__init__.py   |   1 +
 swh/vault/cookers/git_bare.py   |  94 +++++++++-
 swh/vault/tests/test_cookers.py | 378 ++++++++++++++++++++++++++++++++++------
 3 files changed, 413 insertions(+), 60 deletions(-)
Changes applied before test
commit 6c5f0746228febc14b3445451f4645ae46409994
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Jul 16 14:25:35 2021 +0200

    git_bare: Add support for annotated tags pointing to commits

commit 4cb80723d8fbd094bd5b6439d71cfc2e2e091a86
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Jul 15 19:46:54 2021 +0200

    git_bare: Add partial support for snapshots (no release or swh-graph support yet)

commit 70fc50f40b8bc13b022f96be0027e89175773fd7
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Jul 15 17:20:44 2021 +0200

    tests: Split RepoFixtures off TestRevisionCooker
    
    So the 'load' and 'check' parts of the tests can be reused by future
    snapshot tests.

commit a1f42284c683ec1d0ee169cda67f507529822473
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Jul 15 16:58:55 2021 +0200

    tests: Make TestRevisionCooker more modular
    
    So its 'load' and 'check' parts can be reused for testing snapshots

Link to build: https://jenkins.softwareheritage.org/job/DVAU/job/tests-on-diff/140/
See console output for more information: https://jenkins.softwareheritage.org/job/DVAU/job/tests-on-diff/140/console

Harbormaster returned this revision to the author for changes because remote builds failed.Jul 16 2021, 2:50 PM
Harbormaster failed remote builds in B22633: Diff 21663!
vlorentz updated this revision to Diff 21665.Jul 16 2021, 3:22 PM

really fix this time.

swh-public-ci added a comment.Jul 16 2021, 3:25 PM

Build is green

Patch application report for D6000 (id=21665)

Could not rebase; Attempt merge onto 5951a5c63b...

Updating 5951a5c..73b2b13
Fast-forward
 swh/vault/cookers/__init__.py   |   1 +
 swh/vault/cookers/git_bare.py   |  94 +++++++++-
 swh/vault/tests/test_cookers.py | 378 ++++++++++++++++++++++++++++++++++------
 3 files changed, 413 insertions(+), 60 deletions(-)
Changes applied before test
commit 73b2b13b9e06a8bdf5f6ea5d3b526d1e0c53fcf9
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Fri Jul 16 14:25:35 2021 +0200

    git_bare: Add support for annotated tags pointing to commits

commit 4cb80723d8fbd094bd5b6439d71cfc2e2e091a86
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Jul 15 19:46:54 2021 +0200

    git_bare: Add partial support for snapshots (no release or swh-graph support yet)

commit 70fc50f40b8bc13b022f96be0027e89175773fd7
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Jul 15 17:20:44 2021 +0200

    tests: Split RepoFixtures off TestRevisionCooker
    
    So the 'load' and 'check' parts of the tests can be reused by future
    snapshot tests.

commit a1f42284c683ec1d0ee169cda67f507529822473
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Jul 15 16:58:55 2021 +0200

    tests: Make TestRevisionCooker more modular
    
    So its 'load' and 'check' parts can be reused for testing snapshots

See https://jenkins.softwareheritage.org/job/DVAU/job/tests-on-diff/142/ for more details.

Harbormaster completed remote builds in B22635: Diff 21665.Jul 16 2021, 3:25 PM
vlorentz requested review of this revision.Jul 16 2021, 3:25 PM
vlorentz added a child revision: D6002: git_bare: Add support for swh-graph when loading a snapshot.Jul 19 2021, 10:48 AM
ardumont accepted this revision.Jul 19 2021, 2:27 PM
This revision is now accepted and ready to land.Jul 19 2021, 2:27 PM
Closed by commit rDVAU73b2b13b9e06: git_bare: Add support for annotated tags pointing to commits (authored by vlorentz). · Explain WhyJul 26 2021, 2:30 PM
This revision was automatically updated to reflect the committed changes.
vlorentz added a commit: rDVAU73b2b13b9e06: git_bare: Add support for annotated tags pointing to commits.
vlorentz added a task: T843: Vault: Add a "git bare" tarball cooker.Aug 2 2021, 1:56 PM

Revision Contents
Changeset List

PathSize
swh/
vault/
cookers/
28 lines
tests/
81 lines

Diff 21663

View Options

swh/vault/cookers/git_bare.py

Show First 20 Lines • Show All 565 Lines • ▼ Show 20 Lines def test_content_add_collision(self):
cont1b = cont1.copy() cont1b = cont1.copy()
sha256_array = bytearray(cont1b['sha256']) sha256_array = bytearray(cont1b['sha256'])
sha256_array[0] += 1 sha256_array[0] += 1
cont1b['sha256'] = bytes(sha256_array) cont1b['sha256'] = bytes(sha256_array)
with self.assertRaises(HashCollision) as cm: with self.assertRaises(HashCollision) as cm:
self.storage.content_add([cont1, cont1b]) self.storage.content_add([cont1, cont1b])
self.assertEqual(cm.exception.args[0], 'sha1') self.assertIn(cm.exception.args[0], ['sha1', 'sha1_git', 'blake2s256'])
def test_skipped_content_add(self): def test_skipped_content_add(self):
cont = self.skipped_cont.copy() cont = self.skipped_cont.copy()
cont2 = self.skipped_cont2.copy() cont2 = self.skipped_cont2.copy()
cont2['blake2s256'] = None cont2['blake2s256'] = None
self.storage.content_add([cont, cont, cont2]) self.storage.content_add([cont, cont, cont2])
▲ Show 20 Lines • Show All 194 Lines • ▼ Show 20 Lines def test_revision_log(self):
self.storage.revision_add([self.revision3, self.storage.revision_add([self.revision3,
self.revision4]) self.revision4])
# when # when
actual_results = list(self.storage.revision_log( actual_results = list(self.storage.revision_log(
[self.revision4['id']])) [self.revision4['id']]))
# hack: ids generated # hack: ids generated
for actual_result in actual_results: for actual_result in actual_results:
if 'id' in actual_result['author']:
del actual_result['author']['id'] del actual_result['author']['id']
if 'id' in actual_result['committer']:
del actual_result['committer']['id'] del actual_result['committer']['id']
douarddaUnsubmitted
Not Done
Inline Actions

These hunks (in test_storage.py) should not be in this Diff. Having to do so in order to make your test_in_memory pass (I guess it's the reason for these modifications) prove your implementation does not respect the "API" of the original storage class.

It's probably no big deal here, but it's a bad practice...

douardda: These hunks (in test_storage.py) should not be in this Diff. Having to do so in order to make…
vlorentzAuthorUnsubmitted
Done
Inline Actions

comments like # hack: ids generated show that these IDs are quirks from the SQL backend, not features.

vlorentz: comments like `# hack: ids generated` show that these IDs are quirks from the SQL backend, not…
vlorentzAuthorUnsubmitted
Done
Inline Actions

Actually, according to Storage's docstrings, actual_result['author'] and actual_result['committer'] should not even exist.

vlorentz: Actually, according to Storage's docstrings, `actual_result['author']` and `actual_result…
self.assertEqual(len(actual_results), 2) # rev4 -child-> rev3 self.assertEqual(len(actual_results), 2) # rev4 -child-> rev3
self.assertEqual(actual_results[0], self.assertEqual(actual_results[0],
self.normalize_entity(self.revision4)) self.normalize_entity(self.revision4))
self.assertEqual(actual_results[1], self.assertEqual(actual_results[1],
self.normalize_entity(self.revision3)) self.normalize_entity(self.revision3))
def test_revision_log_with_limit(self): def test_revision_log_with_limit(self):
# given # given
# self.revision4 -is-child-of-> self.revision3 # self.revision4 -is-child-of-> self.revision3
self.storage.revision_add([self.revision3, self.storage.revision_add([self.revision3,
self.revision4]) self.revision4])
actual_results = list(self.storage.revision_log( actual_results = list(self.storage.revision_log(
[self.revision4['id']], 1)) [self.revision4['id']], 1))
# hack: ids generated # hack: ids generated
for actual_result in actual_results: for actual_result in actual_results:
if 'id' in actual_result['author']:
del actual_result['author']['id'] del actual_result['author']['id']
if 'id' in actual_result['committer']:
del actual_result['committer']['id'] del actual_result['committer']['id']
self.assertEqual(len(actual_results), 1) self.assertEqual(len(actual_results), 1)
self.assertEqual(actual_results[0], self.revision4) self.assertEqual(actual_results[0], self.revision4)
@staticmethod @staticmethod
def _short_revision(revision): def _short_revision(revision):
return [revision['id'], revision['parents']] return [revision['id'], revision['parents']]
Show All 27 Linesclass CommonTestStorage(TestStorageData):
def test_revision_get(self): def test_revision_get(self):
self.storage.revision_add([self.revision]) self.storage.revision_add([self.revision])
actual_revisions = list(self.storage.revision_get( actual_revisions = list(self.storage.revision_get(
[self.revision['id'], self.revision2['id']])) [self.revision['id'], self.revision2['id']]))
# when # when
if 'id' in actual_revisions[0]['author']:
del actual_revisions[0]['author']['id'] # hack: ids are generated del actual_revisions[0]['author']['id'] # hack: ids are generated
if 'id' in actual_revisions[0]['committer']:
del actual_revisions[0]['committer']['id'] del actual_revisions[0]['committer']['id']
self.assertEqual(len(actual_revisions), 2) self.assertEqual(len(actual_revisions), 2)
self.assertEqual(actual_revisions[0], self.assertEqual(actual_revisions[0],
self.normalize_entity(self.revision)) self.normalize_entity(self.revision))
self.assertIsNone(actual_revisions[1]) self.assertIsNone(actual_revisions[1])
def test_revision_get_no_parents(self): def test_revision_get_no_parents(self):
self.storage.revision_add([self.revision3]) self.storage.revision_add([self.revision3])
▲ Show 20 Lines • Show All 1,116 Lines • Show Last 20 Lines
View Options

swh/vault/tests/test_cookers.py

Show First 20 Lines • Show All 565 Lines • ▼ Show 20 Lines def test_content_add_collision(self):
cont1b = cont1.copy() cont1b = cont1.copy()
sha256_array = bytearray(cont1b['sha256']) sha256_array = bytearray(cont1b['sha256'])
sha256_array[0] += 1 sha256_array[0] += 1
cont1b['sha256'] = bytes(sha256_array) cont1b['sha256'] = bytes(sha256_array)
with self.assertRaises(HashCollision) as cm: with self.assertRaises(HashCollision) as cm:
self.storage.content_add([cont1, cont1b]) self.storage.content_add([cont1, cont1b])
self.assertEqual(cm.exception.args[0], 'sha1') self.assertIn(cm.exception.args[0], ['sha1', 'sha1_git', 'blake2s256'])
def test_skipped_content_add(self): def test_skipped_content_add(self):
cont = self.skipped_cont.copy() cont = self.skipped_cont.copy()
cont2 = self.skipped_cont2.copy() cont2 = self.skipped_cont2.copy()
cont2['blake2s256'] = None cont2['blake2s256'] = None
self.storage.content_add([cont, cont, cont2]) self.storage.content_add([cont, cont, cont2])
▲ Show 20 Lines • Show All 194 Lines • ▼ Show 20 Lines def test_revision_log(self):
self.storage.revision_add([self.revision3, self.storage.revision_add([self.revision3,
self.revision4]) self.revision4])
# when # when
actual_results = list(self.storage.revision_log( actual_results = list(self.storage.revision_log(
[self.revision4['id']])) [self.revision4['id']]))
# hack: ids generated # hack: ids generated
for actual_result in actual_results: for actual_result in actual_results:
if 'id' in actual_result['author']:
del actual_result['author']['id'] del actual_result['author']['id']
if 'id' in actual_result['committer']:
del actual_result['committer']['id'] del actual_result['committer']['id']
douarddaUnsubmitted
Not Done
Inline Actions

These hunks (in test_storage.py) should not be in this Diff. Having to do so in order to make your test_in_memory pass (I guess it's the reason for these modifications) prove your implementation does not respect the "API" of the original storage class.

It's probably no big deal here, but it's a bad practice...

douardda: These hunks (in test_storage.py) should not be in this Diff. Having to do so in order to make…
vlorentzAuthorUnsubmitted
Done
Inline Actions

comments like # hack: ids generated show that these IDs are quirks from the SQL backend, not features.

vlorentz: comments like `# hack: ids generated` show that these IDs are quirks from the SQL backend, not…
vlorentzAuthorUnsubmitted
Done
Inline Actions

Actually, according to Storage's docstrings, actual_result['author'] and actual_result['committer'] should not even exist.

vlorentz: Actually, according to Storage's docstrings, `actual_result['author']` and `actual_result…
self.assertEqual(len(actual_results), 2) # rev4 -child-> rev3 self.assertEqual(len(actual_results), 2) # rev4 -child-> rev3
self.assertEqual(actual_results[0], self.assertEqual(actual_results[0],
self.normalize_entity(self.revision4)) self.normalize_entity(self.revision4))
self.assertEqual(actual_results[1], self.assertEqual(actual_results[1],
self.normalize_entity(self.revision3)) self.normalize_entity(self.revision3))
def test_revision_log_with_limit(self): def test_revision_log_with_limit(self):
# given # given
# self.revision4 -is-child-of-> self.revision3 # self.revision4 -is-child-of-> self.revision3
self.storage.revision_add([self.revision3, self.storage.revision_add([self.revision3,
self.revision4]) self.revision4])
actual_results = list(self.storage.revision_log( actual_results = list(self.storage.revision_log(
[self.revision4['id']], 1)) [self.revision4['id']], 1))
# hack: ids generated # hack: ids generated
for actual_result in actual_results: for actual_result in actual_results:
if 'id' in actual_result['author']:
del actual_result['author']['id'] del actual_result['author']['id']
if 'id' in actual_result['committer']:
del actual_result['committer']['id'] del actual_result['committer']['id']
self.assertEqual(len(actual_results), 1) self.assertEqual(len(actual_results), 1)
self.assertEqual(actual_results[0], self.revision4) self.assertEqual(actual_results[0], self.revision4)
@staticmethod @staticmethod
def _short_revision(revision): def _short_revision(revision):
return [revision['id'], revision['parents']] return [revision['id'], revision['parents']]
Show All 27 Linesclass CommonTestStorage(TestStorageData):
def test_revision_get(self): def test_revision_get(self):
self.storage.revision_add([self.revision]) self.storage.revision_add([self.revision])
actual_revisions = list(self.storage.revision_get( actual_revisions = list(self.storage.revision_get(
[self.revision['id'], self.revision2['id']])) [self.revision['id'], self.revision2['id']]))
# when # when
if 'id' in actual_revisions[0]['author']:
del actual_revisions[0]['author']['id'] # hack: ids are generated del actual_revisions[0]['author']['id'] # hack: ids are generated
if 'id' in actual_revisions[0]['committer']:
del actual_revisions[0]['committer']['id'] del actual_revisions[0]['committer']['id']
self.assertEqual(len(actual_revisions), 2) self.assertEqual(len(actual_revisions), 2)
self.assertEqual(actual_revisions[0], self.assertEqual(actual_revisions[0],
self.normalize_entity(self.revision)) self.normalize_entity(self.revision))
self.assertIsNone(actual_revisions[1]) self.assertIsNone(actual_revisions[1])
def test_revision_get_no_parents(self): def test_revision_get_no_parents(self):
self.storage.revision_add([self.revision3]) self.storage.revision_add([self.revision3])
▲ Show 20 Lines • Show All 1,116 Lines • Show Last 20 Lines
About · Developer information · Legal