diff --git a/components/libraryprovider.py b/components/libraryprovider.py index 656a6b9..c505901 100644 --- a/components/libraryprovider.py +++ b/components/libraryprovider.py @@ -197,6 +197,18 @@ def get_libraries(self, gecko_path): return self._libraries + def library_has_patches(self, gecko_path, library): + # Re-read the moz.yaml rather than trusting library.has_patches, which + # reflects the file as it was when the run started (the AI conflict + # resolver may since have removed every patch). + yaml_path = os.path.join(gecko_path, library.yaml_path) + with open(yaml_path, "r") as mozyaml: + contents = yaml.load(mozyaml.read(), Loader=yaml.BaseLoader) + # A moz.yaml without a vendoring section is broken, not patch-free. + if not isinstance(contents, dict) or not isinstance(contents.get('vendoring'), dict): + raise Exception("%s has no vendoring section" % yaml_path) + return not not contents['vendoring'].get('patches') + @staticmethod def validate_library(yaml_contents, yaml_path): library = yaml.load(yaml_contents, Loader=yaml.BaseLoader) diff --git a/tasktypes/vendoring.py b/tasktypes/vendoring.py index be929c7..bcbf71e 100644 --- a/tasktypes/vendoring.py +++ b/tasktypes/vendoring.py @@ -186,6 +186,9 @@ def _process_new_job(self, library, task, new_version, timestamp, most_recent_jo raise e ai_resolved_conflicts = False + # Whether there's a separate commit applying the mozilla patches. Normally + # the same as library.has_patches, but the AI may remove every patch. + has_patches_commit = library.has_patches if library.has_patches: # Apply Patches ------------------- try: @@ -217,9 +220,18 @@ def _process_new_job(self, library, task, new_version, timestamp, most_recent_jo CommentTemplates.AI_RESOLVED_PATCH_CONFLICTS(outcome, ai_details), needinfo=needinfo) - # Re-apply the now-updated patches so the vendored tree is patched. try: - self.vendorProvider.patch(library, new_version) + # If the AI removed every patch (e.g. because they were all + # upstreamed), there is nothing left to re-apply or commit. + # Re-reading the moz.yaml is inside this handler because the + # AI may have left it broken. + has_patches_commit = self.libraryProvider.library_has_patches( + self.config['General'].get('gecko-path'), library) + if has_patches_commit: + # Re-apply the now-updated patches so the vendored tree is patched. + self.vendorProvider.patch(library, new_version) + else: + self.logger.log("The AI removed every mozilla patch, so there are none to re-apply or commit.", level=LogLevel.Info) except Exception as e2: self.dbProvider.update_job_status(created_job, JOBSTATUS.DONE, JOBOUTCOME.COULD_NOT_PATCH) self.bugzillaProvider.comment_on_bug(created_job.bugzilla_id, CommentTemplates.COULD_NOT_GENERAL_ERROR("apply the mozilla patches even after AI conflict resolution.", errormessage=_patch_exception_message(e2)), needinfo=library.maintainer_bz) @@ -228,6 +240,7 @@ def _process_new_job(self, library, task, new_version, timestamp, most_recent_jo # The AI added its own commit for the .patch/moz.yaml fixes, so # there is now an extra commit to submit to Phabricator. ai_resolved_conflicts = True + if has_patches_commit: # Commit Patches ------------------ try: self.mercurialProvider.commit_patches(library, created_job.bugzilla_id, new_version) @@ -254,14 +267,14 @@ def _process_new_job(self, library, task, new_version, timestamp, most_recent_jo try: # One commit for the vendoring, plus one for the patches (if any), # plus one more if the AI added a commit resolving patch conflicts. - num_commits = 1 + (1 if library.has_patches else 0) + (1 if ai_resolved_conflicts else 0) + num_commits = 1 + (1 if has_patches_commit else 0) + (1 if ai_resolved_conflicts else 0) phab_revisions = self.phabricatorProvider.submit_patches(created_job.bugzilla_id, num_commits) assert len(phab_revisions) == num_commits, "We don't have the correct number of phabricator patches; we have %s, expected %s" % (len(phab_revisions), num_commits) # Revisions come back bottom-up, matching the order the commits were made. purposes = ['vendoring commit'] if ai_resolved_conflicts: purposes.append('patch conflict resolution commit') - if library.has_patches: + if has_patches_commit: purposes.append('patches commit') for revision, purpose in zip(phab_revisions, purposes): self.dbProvider.add_phab_revision(created_job, revision, purpose) diff --git a/tests/functionality_all_platforms.py b/tests/functionality_all_platforms.py index 9f45184..a69d270 100755 --- a/tests/functionality_all_platforms.py +++ b/tests/functionality_all_platforms.py @@ -470,6 +470,83 @@ def phab_submit(): finally: self._cleanup(u, expected_values) + # patch fails -> AI resolves it by removing every patch (e.g. all upstreamed) -> + # nothing to re-apply or commit, so only the vendoring and AI commits go to phabricator + @logEntryExitHeaderLine + def testPatchConflictAIRemovedAllPatches(self): + patch_calls = [0] + + def patch_callback(): + patch_calls[0] += 1 + if patch_calls[0] == 1: + raise_(Exception("Could not apply mozilla.patch, possible reasons:")) + raise_(Exception("Patch import was specified but there are no vendored patches defined.")) + + arc_calls = [0] + + def phab_submit(): + arc_calls[0] += 1 + v = 92000 + arc_calls[0] + return ARC_OUTPUT % (v, v) + + library_filter = 'png' + (u, expected_values, _check_jobs) = self._setup( + library_filter, + lambda b: ["try_rev|2021-02-09 15:30:04 -0500|2021-02-12 17:40:01 +0000"], + lambda: 55, # get_filed_bug_id_func + lambda b: {}, # filed_bug_ids_func + AssertFalse, # treeherder_response + command_callbacks={'patch': patch_callback, 'phab_submit': phab_submit}, + ai_config={'ai_outcome': 'trivial success', + 'ai_details': ['The only patch is already applied upstream, so I removed it.']}, + ) + u.libraryProvider.config['patches_after_ai'] = False + try: + u.run(library_filter=library_filter) + + self.assertEqual(u.aiProvider.resolve_call_count, 1, "The AI conflict resolver should have been called once") + self.assertEqual(patch_calls[0], 1, "With no patches left, they should not have been re-applied") + + lib = [lib for lib in u.libraryProvider.get_libraries(u.config_dictionary['General']['gecko-path']) if library_filter in lib.name][0] + j = u.dbProvider.get_job(lib, expected_values.library_new_version_id()) + self.assertEqual(JOBSTATUS.AWAITING_SECOND_PLATFORMS_TRY_RESULTS, j.status) + self.assertEqual(JOBOUTCOME.PENDING, j.outcome) + self.assertEqual([p.purpose for p in j.phab_revisions], + ['vendoring commit', 'patch conflict resolution commit']) + finally: + self._cleanup(u, expected_values) + + # patch fails -> AI claims success -> but leaves the moz.yaml unreadable -> COULD_NOT_PATCH + @logEntryExitHeaderLine + def testPatchConflictAILeftMozYamlBroken(self): + patch_calls = [0] + + def patch_callback(): + patch_calls[0] += 1 + raise_(Exception("Could not apply mozilla.patch, possible reasons:")) + + library_filter = 'png' + (u, expected_values, _check_jobs) = self._setup( + library_filter, + lambda b: ["try_rev|2021-02-09 15:30:04 -0500|2021-02-12 17:40:01 +0000"], + lambda: 57, # get_filed_bug_id_func + lambda b: {}, # filed_bug_ids_func + AssertFalse, # treeherder_response + command_callbacks={'patch': patch_callback}, + ai_config={'ai_outcome': 'trivial success', 'ai_details': ['I edited the moz.yaml.']}, + ) + u.libraryProvider.config['patches_after_ai'] = 'broken' + try: + u.run(library_filter=library_filter) + self.assertEqual(u.aiProvider.resolve_call_count, 1) + self.assertEqual(patch_calls[0], 1, "The patches should not be re-applied when the moz.yaml can't be read") + lib = [lib for lib in u.libraryProvider.get_libraries(u.config_dictionary['General']['gecko-path']) if library_filter in lib.name][0] + j = u.dbProvider.get_job(lib, expected_values.library_new_version_id()) + self.assertEqual(JOBSTATUS.DONE, j.status) + self.assertEqual(JOBOUTCOME.COULD_NOT_PATCH, j.outcome) + finally: + self._cleanup(u, expected_values) + # patch fails -> AI claims success -> but the patches STILL fail to re-apply -> COULD_NOT_PATCH @logEntryExitHeaderLine def testPatchConflictAIResolvedButReapplyStillFails(self): diff --git a/tests/library.py b/tests/library.py index ae50ac2..3405c77 100755 --- a/tests/library.py +++ b/tests/library.py @@ -7,11 +7,13 @@ import sys import unittest import os +import tempfile sys.path.append(".") sys.path.append("..") from components.libraryprovider import LibraryProvider, Library +from components.utilities import Struct from tests.mock_commandprovider import TestCommandProvider from components.logging import SimpleLoggerConfig @@ -150,6 +152,22 @@ def setUpClass(cls): }) cls.libraryprovider.update_config(additional_config) + def testLibraryHasPatches(self): + def has_patches(contents): + with tempfile.TemporaryDirectory() as gecko: + if contents is not None: + with open(os.path.join(gecko, "moz.yaml"), "w") as f: + f.write(contents) + return self.libraryprovider.library_has_patches(gecko, Struct(yaml_path='moz.yaml')) + + self.assertTrue(has_patches("vendoring:\n url: https://example.invalid\n patches:\n - mozilla.patch\n")) + self.assertFalse(has_patches("vendoring:\n url: https://example.invalid\n patches: []\n")) + self.assertFalse(has_patches("vendoring:\n url: https://example.invalid\n")) + # A broken moz.yaml is an error, not a library without patches. + for broken in ["origin:\n name: foo\n", "", "vendoring: [unclosed\n", None]: + with self.assertRaises(Exception, msg=repr(broken)): + has_patches(broken) + def testLibraryFindAndImport(self): libs = self.libraryprovider.get_libraries(os.getcwd()) self.maxDiff = 999999 diff --git a/tests/mock_libraryprovider.py b/tests/mock_libraryprovider.py index 08f679a..88609ae 100644 --- a/tests/mock_libraryprovider.py +++ b/tests/mock_libraryprovider.py @@ -18,6 +18,13 @@ class MockLibraryProvider(BaseProvider, INeedsCommandProvider, INeedsLoggingProv def __init__(self, config): self.config = config + def library_has_patches(self, gecko_path, library): + # Tests set 'patches_after_ai' to simulate the AI removing every patch, + # or to 'broken' to simulate it leaving the moz.yaml unreadable. + if self.config.get('patches_after_ai') == 'broken': + raise Exception("moz.yaml has no vendoring section") + return self.config.get('patches_after_ai', library.has_patches) + def get_libraries(self, gecko_path): return [ Library({