Automatically commit blacklisted ports

oca_port/__init__.py

@@ -105,10 +105,10 @@ def main(
         to_branch = misc.Branch(repo, to_branch, default_remote=upstream)
     except ValueError as exc:
         _check_remote(repo_name, *exc.args)
-    storage = misc.InputStorage(repo.working_dir)
     _fetch_branches(from_branch, to_branch, verbose=verbose)
     _check_branches(from_branch, to_branch)
     _check_addon_exists(addon, from_branch, raise_exc=True)
+    storage = misc.InputStorage(to_branch, addon)
     # Check if the addon (folder) exists on the target branch
     #   - if it already exists, check if some PRs could be ported
     if _check_addon_exists(addon, to_branch):

oca_port/migrate_addon.py

@@ -46,6 +46,17 @@
     "\t4) Create the PR against {upstream_org}/{repo_name}:",
     f"\t\t=> {bc.BOLD}" "{new_pr_url}" f"{bc.END}",
 ])
+BLACKLIST_TIPS = "\n".join([
+    f"\n{bc.BOLD}{bc.OKCYAN}The next steps are:{bc.END}",
+    (
+        "\t1) On a shell command, type this for uploading the content to GitHub:\n"
+        f"{bc.DIM}"
+        "\t\t$ git push {fork} {mig_branch} --set-upstream"
+        f"{bc.END}"
+    ),
+    "\t2) Create the PR against {upstream_org}/{repo_name}:",
+    f"\t\t=> {bc.BOLD}" "{new_pr_url}" f"{bc.END}",
+])
 
 
 class MigrateAddon():
@@ -69,12 +80,12 @@ def __init__(
         self.non_interactive = non_interactive
 
     def run(self):
-        if self.storage.is_addon_blacklisted(
-                self.from_branch.name, self.to_branch.name, self.addon
-                ):
+        blacklisted = self.storage.is_addon_blacklisted()
+        if blacklisted:
             print(
-                f"{bc.DIM}Migration to {self.to_branch.name} for "
-                f"{bc.BOLD}{self.addon}{bc.END} {bc.DIM}blacklisted{bc.ENDD}")
+                f"{bc.DIM}Migration of {bc.BOLD}{self.addon}{bc.END} "
+                f"{bc.DIM}to {self.to_branch.name} "
+                f"blacklisted ({blacklisted}){bc.ENDD}")
             return
         if self.non_interactive:
             # Exit with an error code if the addon is eligible for a migration
@@ -85,11 +96,9 @@ def run(self):
             f"to {bc.BOLD}{self.to_branch.name}{bc.END}?"
         )
         if not click.confirm(confirm):
-            self.storage.blacklist_addon(
-                self.from_branch.name, self.to_branch.name,
-                self.addon, confirm=True
-            )
-            return
+            self.storage.blacklist_addon(confirm=True)
+            if not self.storage.dirty:
+                return
         # Check if a migration PR already exists
         # TODO
         if not self.fork:
@@ -98,6 +107,11 @@ def run(self):
             raise click.ClickException("Untracked files detected, abort")
         self._checkout_base_branch()
         if self._create_mig_branch():
+            # Case where the addon shouldn't be ported (blacklisted)
+            if self.storage.dirty:
+                self.storage.commit()
+                self._print_tips(blacklisted=True)
+                return
             with tempfile.TemporaryDirectory() as patches_dir:
                 self._generate_patches(patches_dir)
                 self._apply_patches(patches_dir)
@@ -178,7 +192,7 @@ def _run_pre_commit(self):
                 "-m", f"[IMP] {self.addon}: black, isort, prettier", "--no-verify"
             )
 
-    def _print_tips(self):
+    def _print_tips(self, blacklisted=False):
         mig_tasks_url = MIG_TASKS_URL.format(branch=self.to_branch.name)
         pr_title_encoded = urllib.parse.quote(
             MIG_NEW_PR_TITLE.format(to_branch=self.to_branch.name[:4], addon=self.addon)
@@ -188,6 +202,14 @@ def _print_tips(self):
             to_branch=self.to_branch.name, user_org=self.user_org,
             mig_branch=self.mig_branch.name, title=pr_title_encoded
         )
+        if blacklisted:
+            tips = BLACKLIST_TIPS.format(
+                upstream_org=self.upstream_org, repo_name=self.repo_name,
+                fork=self.fork, mig_branch=self.mig_branch.name,
+                new_pr_url=new_pr_url
+            )
+            print(tips)
+            return
         tips = MIG_TIPS.format(
             upstream_org=self.upstream_org, repo_name=self.repo_name,
             addon=self.addon, to_branch=self.to_branch.name, fork=self.fork,

oca_port/misc.py

@@ -249,23 +249,40 @@ class InputStorage():
 
     If commits/pull requests of an addon may be ported, some of them could be
     false-positive and as such the user can choose to not port them.
-    This class will help to store these informations and by doing so the tool
-    won't list anymore these commits the next time we perform an analysis on
-    the same addon.
+    This class will help to store these informations so the tool won't list
+    anymore these commits the next time we perform an analysis on the same addon.
 
-    Technically the data are stored in a json file at the root of the repository.
+    Technically the data are stored in one JSON file per addon in a hidden
+    folder at the root of the repository. E.g:
+
+        {
+          "no_migration": "included in standard"
+        }
+
+    Or:
+
+        {
+          "pull_requests": {
+            490: "lint changes"
+          }
+        }
     """
-    def __init__(self, root_path):
-        self.root_path = root_path
+    storage_dirname = ".oca/oca-port"
+
+    def __init__(self, to_branch, addon):
+        self.to_branch = to_branch
+        self.repo = self.to_branch.repo
+        self.root_path = self.repo.working_dir
+        self.addon = addon
         self._data = self._get_data()
+        self.dirty = False
 
     def _get_data(self):
         """Return the data of the current repository.
 
         If a JSON file is found, return its content, otherwise return an empty
         dictionary.
         """
-        file_path = self._get_file_path()
 
         def defaultdict_from_dict(d):
             nd = lambda: defaultdict(nd)    # noqa
@@ -274,51 +291,77 @@ def defaultdict_from_dict(d):
             return ni
 
         try:
-            with open(file_path, "r") as file_:
-                return json.load(file_, object_hook=defaultdict_from_dict)
-        except IOError:
+            # Read the JSON file from 'to_branch'
+            tree = self.repo.commit(self.to_branch.ref()).tree
+            blob = tree/self.storage_dirname/"blacklist"/f"{self.addon}.json"
+            content = blob.data_stream.read().decode()
+            return json.loads(content, object_hook=defaultdict_from_dict)
+        except KeyError:
             nested_dict = lambda: defaultdict(nested_dict)  # noqa
             return nested_dict()
 
     def save(self):
         """Store the data at the root of the current repository."""
-        if not self._data:
-            return
+        if not self._data or not self.dirty:
+            return False
         file_path = self._get_file_path()
+        os.makedirs(os.path.dirname(file_path), exist_ok=True)
         with open(file_path, "w") as file_:
-            json.dump(self._data, file_, indent=4)
+            json.dump(self._data, file_, indent=2)
+        return True
 
     def _get_file_path(self):
-        return os.path.join(self.root_path, ".oca-port.json")
+        return os.path.join(
+            self.root_path, self.storage_dirname, "blacklist", f"{self.addon}.json"
+        )
 
-    def is_pr_blacklisted(self, from_branch, to_branch, addon, pr_ref):
+    def is_pr_blacklisted(self, pr_ref):
         pr_ref = str(pr_ref or "orphaned_commits")
-        return self._data.get(
-            from_branch, {}).get(
-                to_branch, {}).get(
-                    addon, {}).get(
-                        "blacklist_pull_requests", {}).get(pr_ref, False)
-
-    def blacklist_pr(self, from_branch, to_branch, addon, pr_ref, confirm=False):
-        if confirm and not click.confirm("\tRemember this choice?"):
+        return self._data.get("pull_requests", {}).get(pr_ref, False)
+
+    def blacklist_pr(self, pr_ref, confirm=False, reason=None):
+        if confirm and not click.confirm("\tBlacklist this PR?"):
             return
+        if not reason:
+            reason = click.prompt("\tReason", type=str)
         pr_ref = str(pr_ref or "orphaned_commits")
-        addon_data = self._data[from_branch][to_branch][addon]
-        addon_data["blacklist_pull_requests"][pr_ref] = True
-        self.save()
-
-    def is_addon_blacklisted(self, from_branch, to_branch, addon):
-        return self._data.get(
-            from_branch, {}).get(
-                to_branch, {}).get(
-                    addon, {}).get("blacklist_addon", False)
-
-    def blacklist_addon(self, from_branch, to_branch, addon, confirm=False):
-        if confirm and not click.confirm("\tRemember this choice?"):
+        self._data["pull_requests"][pr_ref] = reason or "Unknown"
+        self.dirty = True
+
+    def is_addon_blacklisted(self):
+        entry = self._data.get("no_migration")
+        if entry:
+            return entry
+        return False
+
+    def blacklist_addon(self, confirm=False, reason=None):
+        if confirm and not click.confirm("\tBlacklist this module?"):
             return
-        addon_data = self._data[from_branch][to_branch][addon]
-        addon_data["blacklist_addon"] = True
-        self.save()
+        if not reason:
+            reason = click.prompt("Reason", type=str)
+        self._data["no_migration"] = reason or "Unknown"
+        self.dirty = True
+
+    def commit(self):
+        """Commit all files contained in the storage directory."""
+        if not self.save():
+            return
+        if self.repo.is_dirty():
+            raise click.ClickException(
+                "changes not committed detected in this repository."
+            )
+        # Ensure to be on a dedicated branch
+        if self.repo.active_branch.name == self.to_branch.name:
+            raise click.ClickException(
+                "performing commit on upstream branch is not allowed."
+            )
+        # Commit all changes under ./.oca-port
+        self.repo.index.add(self.storage_dirname)
+        if self.repo.is_dirty():
+            self.repo.index.commit(
+                f".oca-port: store '{self.addon}' data", skip_hooks=True
+            )
+            self.dirty = False
 
 
 def clean_text(text):

oca_port/port_addon_pr.py

@@ -108,13 +108,17 @@ def _port_pull_requests(self, branches_diff):
                 pr, commits, base_ref, previous_pr, previous_pr_branch,
             )
             if pr_branch:
-                # Check if commits have been ported
+                # Check if commits have been ported.
+                # If none has been ported, blacklist automatically the current PR.
                 if self.repo.commit(pr_branch.ref()) == current_commit:
                     print("\tℹ️  Nothing has been ported, skipping")
                     self.storage.blacklist_pr(
-                        self.from_branch.name, self.to_branch.name,
-                        self.addon, pr.number
+                        pr.number,
+                        confirm=True,
+                        reason=f"(auto) Nothing to port from PR #{pr.number}",
                     )
+                    if self.storage.dirty:
+                        self.storage.commit()
                     msg = (
                         f"\t{bc.DIM}PR #{pr.number} has been"
                         if pr.number else "Orphaned commits have been"
@@ -160,12 +164,11 @@ def _port_pull_request_commits(
             self.repo.git.checkout(
                 "--no-track", "-b", self.to_branch.name, self.to_branch.ref()
             )
+        # Ask the user if he wants to port the PR (or orphaned commits)
         if not click.confirm("\tPort it?" if pr.number else "\tPort them?"):
-            self.storage.blacklist_pr(
-                self.from_branch.name, self.to_branch.name,
-                self.addon, pr.number, confirm=True
-            )
-            return None, based_on_previous
+            self.storage.blacklist_pr(pr.number, confirm=True)
+            if not self.storage.dirty:
+                return None, based_on_previous
         # Create a local branch based on upstream
         if self.create_branch:
             branch_name = PR_BRANCH_NAME.format(
@@ -179,7 +182,7 @@ def _port_pull_request_commits(
                 # the previous PR branch
                 if previous_pr_branch:
                     based_on_previous = self.repo.is_ancestor(
-                        previous_pr_branch, branch_name
+                        previous_pr_branch.name, branch_name
                     )
                 confirm = (
                     f"\tBranch {bc.BOLD}{branch_name}{bc.END} already exists, "
@@ -200,6 +203,10 @@ def _port_pull_request_commits(
             self.repo.git.checkout("--no-track", "-b", branch_name, base_ref.ref())
         else:
             branch_name = self.to_branch.name
+        # If the PR has been blacklisted we need to commit this information
+        if self.storage.dirty:
+            self.storage.commit()
+            return misc.Branch(self.repo, branch_name), based_on_previous
 
         # Cherry-pick commits of the source PR
         for commit in commits:
@@ -593,13 +600,12 @@ def get_commits_diff(self):
         # Do not return blacklisted PR.
         sorted_commits_by_pr = {}
         for pr in sorted(commits_by_pr, key=lambda pr: pr.merged_at or ""):
-            if self.storage.is_pr_blacklisted(
-                    self.from_branch.name, self.to_branch.name, self.path, pr.number
-                    ):
+            blacklisted = self.storage.is_pr_blacklisted(pr.number)
+            if blacklisted:
                 msg = (
                     f"{bc.DIM}PR #{pr.number}"
                     if pr.number else "Orphaned commits"
-                ) + f" blacklisted{bc.ENDD}"
+                ) + f" blacklisted ({blacklisted}){bc.ENDD}"
                 print(msg)
                 continue
             sorted_commits_by_pr[pr] = commits_by_pr[pr]