From a88324d943e62a2dab8ccdf1b3fed12f8c0876a5 Mon Sep 17 00:00:00 2001 From: Matheus Izvekov Date: Sun, 11 Sep 2022 18:14:20 +0200 Subject: [PATCH] patch_diff: Skip closed dependent revisions When scanning dependencies, instead of trying to skip them based on them being landed, skip them based on the status of the revision. Detecting landed revisions is fraught with problems, as there is no good general way to handle reverts. Signed-off-by: Matheus Izvekov --- scripts/patch_diff.py | 48 ++++++------------------------------------- 1 file changed, 6 insertions(+), 42 deletions(-) diff --git a/scripts/patch_diff.py b/scripts/patch_diff.py index 8fca27f..cdaae27 100755 --- a/scripts/patch_diff.py +++ b/scripts/patch_diff.py @@ -31,10 +31,6 @@ from phabricator import Phabricator LLVM_GITHUB_URL = 'ssh://git@github.com/llvm/llvm-project' FORK_REMOTE_URL = 'ssh://git@github.com/llvm-premerge-tests/llvm-project' -"""How far back the script searches in the git history to find Revisions that -have already landed. """ -APPLIED_SCAN_LIMIT = datetime.timedelta(days=90) - class ApplyPatch: """Apply a diff from Phabricator on local working copy. @@ -43,9 +39,9 @@ class ApplyPatch: that have already landed, but could not be identified by `arc patch`. For a given diff_id, this class will get the dependencies listed on Phabricator. - For each dependency D it will check the diff history: - - if D has already landed, skip it. - - If D has not landed, it will download the patch for D and try to apply it locally. + For each dependency D it will check the it's status: + - if D is closed, skip it. + - If D is not closed, it will download the patch for D and try to apply it locally. Once this class has applied all dependencies, it will apply the original diff. This script must be called from the root folder of a local checkout of @@ -95,13 +91,12 @@ class ApplyPatch: self.revision_id = revision['id'] dependencies = self.get_dependencies(revision) dependencies.reverse() # Now revisions will be from oldest to newest. - missing, landed = self.classify_revisions(dependencies) if len(dependencies) > 0: logging.info('This diff depends on: {}'.format(revision_list_to_str(dependencies))) - logging.info(' Already landed: {}'.format(revision_list_to_str(landed))) - logging.info(' Will be applied: {}'.format(revision_list_to_str(missing))) plan = [] - for r in missing: + for r in dependencies: + if r['status']['value'] is 'closed': + continue d = self.get_diff(r['diffs'][0]) plan.append((r, d)) plan.append((revision, diff)) @@ -294,37 +289,6 @@ class ApplyPatch: def get_raw_diff(self, diff_id: str) -> str: return self.phab.differential.getrawdiff(diffID=diff_id).response - def get_landed_revisions(self): - """Get list of landed revisions from current git branch.""" - diff_regex = re.compile(r'^Differential Revision: https://reviews\.llvm\.org/(.*)$', re.MULTILINE) - earliest_commit = None - rev = self.base_revision - age_limit = datetime.datetime.now() - APPLIED_SCAN_LIMIT - if rev == 'auto': # FIXME: use revison that created the branch - rev = 'main' - for commit in self.repo.iter_commits(rev): - if datetime.datetime.fromtimestamp(commit.committed_date) < age_limit: - break - earliest_commit = commit - result = diff_regex.search(commit.message) - if result is not None: - yield result.group(1) - if earliest_commit is not None: - logging.info(f'Earliest analyzed commit in history {earliest_commit.hexsha}, ' - f'{earliest_commit.committed_datetime}') - return - - def classify_revisions(self, revisions: List[Dict]) -> Tuple[List[Dict], List[Dict]]: - """Check which of the dependencies have already landed on the current branch.""" - landed_deps = [] - missing_deps = [] - for d in revisions: - if diff_to_str(d['id']) in self.get_landed_revisions(): - landed_deps.append(d) - else: - missing_deps.append(d) - return missing_deps, landed_deps - def diff_to_str(diff: int) -> str: """Convert a diff id to a string with leading "D"."""