Sync improvement addressing #787

.github/workflows/create-lint-wf.yml

@@ -28,6 +28,8 @@ jobs:
           sudo ln -s /tmp/nextflow/nextflow /usr/local/bin/nextflow
 
       - name: Run nf-core/tools
+        env:
+          GITHUB_AUTH_TOKEN: ${{ secrets.nf_core_bot_auth_token }}
         run: |
           nf-core --log-file log.txt create -n testpipeline -d "This pipeline is for testing" -a "Testing McTestface"
           nf-core --log-file log.txt lint nf-core-testpipeline

CHANGELOG.md

@@ -18,6 +18,7 @@
   * Singularity images in module files are now discovered and fetched
   * Direct downloads of Singularity images in python allowed (much faster than running `singularity pull`)
   * Downloads now work with `$NXF_SINGULARITY_CACHEDIR` so that pipelines sharing containers have efficient downloads
+* changed behaviour of `nf-core sync` command as discussed in [[#787]](https://github.com/nf-core/tools/issues/787)
 
 ### Linting
 

nf_core/sync.py

@@ -67,6 +67,7 @@ def __init__(
         self.pipeline_dir = os.path.abspath(pipeline_dir)
         self.from_branch = from_branch
         self.original_branch = None
+        self.merge_branch = "nf-core-template-merge-{}".format(nf_core.__version__)
         self.made_changes = False
         self.make_pr = make_pr
         self.gh_pr_returned_data = {}
@@ -86,6 +87,7 @@ def sync(self):
 
         self.inspect_sync_dir()
         self.get_wf_config()
+        self.close_open_template_merge_pull_requests()
         self.checkout_template_branch()
         self.delete_template_branch_files()
         self.make_template_pipeline()
@@ -95,6 +97,8 @@ def sync(self):
         if self.made_changes and self.make_pr:
             try:
                 self.push_template_branch()
+                self.create_merge_base_branch()
+                self.push_merge_branch()
                 self.make_pull_request()
             except PullRequestException as e:
                 self.reset_target_dir()
@@ -239,60 +243,31 @@ def push_template_branch(self):
         except git.exc.GitCommandError as e:
             raise PullRequestException("Could not push TEMPLATE branch:\n  {}".format(e))
 
-    def make_pull_request(self):
-        """Create a pull request to a base branch (default: dev),
-        from a head branch (default: TEMPLATE)
-
-        Returns: An instance of class requests.Response
+    def close_open_template_merge_pull_requests(self):
+        """Get all template merging branches (starting with 'nf-core-template-merge-')
+        and check for any open PRs from these branches to the self.from_branch
+        If open PRs are found, add a comment and close them
         """
-        # Check that we know the github username and repo name
         try:
-            assert self.gh_username is not None
-            assert self.gh_repo is not None
-        except AssertionError:
-            raise PullRequestException("Could not find GitHub username and repo name")
-
-        # If we've been asked to make a PR, check that we have the credentials
-        try:
-            assert os.environ.get("GITHUB_AUTH_TOKEN", "") != ""
-        except AssertionError:
-            raise PullRequestException(
-                "Environment variable GITHUB_AUTH_TOKEN not set - cannot make PR\n"
-                "Make a PR at the following URL:\n  https://github.com/{}/compare/{}...TEMPLATE".format(
-                    self.gh_repo, self.original_branch
-                )
-            )
-
-        log.info("Submitting a pull request via the GitHub API")
-
-        pr_title = "Important! Template update for nf-core/tools v{}".format(nf_core.__version__)
-        pr_body_text = (
-            "A new release of the main template in nf-core/tools has just been released. "
-            "This automated pull-request attempts to apply the relevant updates to this pipeline.\n\n"
-            "Please make sure to merge this pull-request as soon as possible. "
-            "Once complete, make a new minor release of your pipeline. "
-            "For instructions on how to merge this PR, please see "
-            "[https://nf-co.re/developers/sync](https://nf-co.re/developers/sync#merging-automated-prs).\n\n"
-            "For more information about this release of [nf-core/tools](https://github.com/nf-core/tools), "
-            "please see the [nf-core/tools v{tag} release page](https://github.com/nf-core/tools/releases/tag/{tag})."
-        ).format(tag=nf_core.__version__)
-
-        # Try to update an existing pull-request
-        if self.update_existing_pull_request(pr_title, pr_body_text) is False:
-            # None found - make a new pull-request
-            self.submit_pull_request(pr_title, pr_body_text)
-
-    def update_existing_pull_request(self, pr_title, pr_body_text):
-        """
-        List existing pull-requests between TEMPLATE and self.from_branch
+            log.info("Checking for open PRs from template merge branches")
+            # Get list of all branches
+            branch_list = [b.name for b in self.repo.branches]
+            # Subset to template merging branches
+            branch_list = [b for b in branch_list if b.startswith("nf-core-template-merge-")]
+            # Check for open PRs and close if found
+            for branch in branch_list:
+                self.close_open_pr(branch)
+        except Exception as e:
+            raise log.error("Could not close open pull requests! {}".format(e))
 
-        If one is found, attempt to update it with a new title and body text
-        If none are found, return False
+    def close_open_pr(self, branch):
+        """Given a branch, check for open PRs from that branch to self.from_branch
+        and close if PRs have been found
         """
-        assert os.environ.get("GITHUB_AUTH_TOKEN", "") != ""
+        log.info("Checking branch: {}".format(branch))
         # Look for existing pull-requests
-        list_prs_url = "https://api.github.com/repos/{}/pulls?head=nf-core:TEMPLATE&base={}".format(
-            self.gh_repo, self.from_branch
+        list_prs_url = "https://api.github.com/repos/{}/pulls?head={}&base={}".format(
+            self.gh_repo, branch, self.from_branch
         )
         r = requests.get(
             url=list_prs_url,
@@ -305,18 +280,25 @@ def update_existing_pull_request(self, pr_title, pr_body_text):
             r_json = r.content
             r_pp = r.content
 
-        # PR worked
         if r.status_code == 200:
             log.debug("GitHub API listing existing PRs:\n{}".format(r_pp))
 
             # No open PRs
             if len(r_json) == 0:
-                log.info("No open PRs found between TEMPLATE and {}".format(self.from_branch))
+                log.info("No open PRs found between {} and {}".format(branch, self.from_branch))
                 return False
 
-            # Update existing PR
+            # Close existing PR
+            pr_title = "Important! Template update for nf-core/tools v{} Closed because outdated!".format(
+                nf_core.__version__
+            )
+            pr_body_text = (
+                "A new release of the main template in nf-core/tools has just been released. "
+                "This automated pull-request attempts to apply the relevant updates to this pipeline.\n\n"
+                "This pull-request is outdated and has been closed. A new pull-request has been created instead."
+            )
             pr_update_api_url = r_json[0]["url"]
-            pr_content = {"title": pr_title, "body": pr_body_text}
+            pr_content = {"state": "closed", "title": pr_title, "body": pr_body_text}
 
             r = requests.patch(
                 url=pr_update_api_url,
@@ -333,53 +315,127 @@ def update_existing_pull_request(self, pr_title, pr_body_text):
             # PR update worked
             if r.status_code == 200:
                 log.debug("GitHub API PR-update worked:\n{}".format(r_pp))
-                log.info("Updated GitHub PR: {}".format(r_json["html_url"]))
+                log.info("Closed GitHub PR: {}".format(r_json["html_url"]))
                 return True
             # Something went wrong
             else:
-                log.warning("Could not update PR ('{}'):\n{}\n{}".format(r.status_code, pr_update_api_url, r_pp))
+                log.warning("Could not close PR ('{}'):\n{}\n{}".format(r.status_code, pr_update_api_url, r_pp))
                 return False
 
         # Something went wrong
         else:
             log.warning("Could not list open PRs ('{}')\n{}\n{}".format(r.status_code, list_prs_url, r_pp))
-            return False
+
+    def create_merge_base_branch(self):
+        """Create a new branch from the updated TEMPLATE branch
+        This branch will then be used to create the PR
+        """
+        # Check if branch exists already
+        branch_list = [b.name for b in self.repo.branches]
+        if self.merge_branch in branch_list:
+            original_merge_branch = self.merge_branch
+            # Try to create new branch with number at the end
+            # If <branch_name>-2 already exists, increase the number until branch is new
+            branch_no = 2
+            self.merge_branch = original_merge_branch + "-" + str(branch_no)
+            while self.merge_branch in branch_list:
+                branch_no += 1
+                self.merge_branch = original_merge_branch + "-" + str(branch_no)
+            log.info(
+                "Branch already existed: '{}', creating branch '{}' instead.".format(
+                    original_merge_branch, self.merge_branch
+                )
+            )
+
+        # Create new branch and checkout
+        log.info("Checking out merge base branch {}".format(self.merge_branch))
+        try:
+            self.repo.create_head(self.merge_branch)
+        except git.exc.GitCommandError:
+            raise SyncException("Could not create new branch '{}'".format(self.merge_branch))
+
+    def push_merge_branch(self):
+        """Push the newly created merge branch to the remote repository"""
+        log.info("Pushing {} branch to remote".format(self.merge_branch))
+        try:
+            origin = self.repo.remote()
+            origin.push(self.merge_branch)
+        except git.exc.GitCommandError as e:
+            raise PullRequestException("Could not push {} branch:\n  {}".format(self.merge_branch, e))
+
+    def make_pull_request(self):
+        """Create a pull request to a base branch (default: dev),
+        from a head branch (default: TEMPLATE)
+
+        Returns: An instance of class requests.Response
+        """
+        # Check that we know the github username and repo name
+        if self.gh_username is None and self.gh_repo is None:
+            raise PullRequestException("Could not find GitHub username and repo name")
+
+        # If we've been asked to make a PR, check that we have the credentials
+        if os.environ.get("GITHUB_AUTH_TOKEN", "") == "":
+            raise PullRequestException(
+                "Environment variable GITHUB_AUTH_TOKEN not set - cannot make PR\n"
+                "Make a PR at the following URL:\n  https://github.com/{}/compare/{}...TEMPLATE".format(
+                    self.gh_repo, self.original_branch
+                )
+            )
+
+        log.info("Submitting a pull request via the GitHub API")
+
+        pr_title = "Important! Template update for nf-core/tools v{}".format(nf_core.__version__)
+        pr_body_text = (
+            "A new release of the main template in nf-core/tools has just been released. "
+            "This automated pull-request attempts to apply the relevant updates to this pipeline.\n\n"
+            "Please make sure to merge this pull-request as soon as possible. "
+            "Once complete, make a new minor release of your pipeline. "
+            "For instructions on how to merge this PR, please see "
+            "[https://nf-co.re/developers/sync](https://nf-co.re/developers/sync#merging-automated-prs).\n\n"
+            "For more information about this release of [nf-core/tools](https://github.com/nf-core/tools), "
+            "please see the [nf-core/tools v{tag} release page](https://github.com/nf-core/tools/releases/tag/{tag})."
+        ).format(tag=nf_core.__version__)
+
+        # Make new pull-request
+        self.submit_pull_request(pr_title, pr_body_text)
 
     def submit_pull_request(self, pr_title, pr_body_text):
         """
         Create a new pull-request on GitHub
         """
-        assert os.environ.get("GITHUB_AUTH_TOKEN", "") != ""
-        pr_content = {
-            "title": pr_title,
-            "body": pr_body_text,
-            "maintainer_can_modify": True,
-            "head": "TEMPLATE",
-            "base": self.from_branch,
-        }
-
-        r = requests.post(
-            url="https://api.github.com/repos/{}/pulls".format(self.gh_repo),
-            data=json.dumps(pr_content),
-            auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ.get("GITHUB_AUTH_TOKEN")),
-        )
-        try:
-            self.gh_pr_returned_data = json.loads(r.content)
-            returned_data_prettyprint = json.dumps(self.gh_pr_returned_data, indent=4)
-        except:
-            self.gh_pr_returned_data = r.content
-            returned_data_prettyprint = r.content
+        if not os.environ.get("GITHUB_AUTH_TOKEN", "") == "":
+            pr_content = {
+                "title": pr_title,
+                "body": pr_body_text,
+                "maintainer_can_modify": True,
+                "head": self.merge_branch,
+                "base": self.from_branch,
+            }
+
+            r = requests.post(
+                url="https://api.github.com/repos/{}/pulls".format(self.gh_repo),
+                data=json.dumps(pr_content),
+                auth=requests.auth.HTTPBasicAuth(self.gh_username, os.environ.get("GITHUB_AUTH_TOKEN")),
+            )
+            try:
+                self.gh_pr_returned_data = json.loads(r.content)
+                returned_data_prettyprint = json.dumps(self.gh_pr_returned_data, indent=4)
+            except:
+                self.gh_pr_returned_data = r.content
+                returned_data_prettyprint = r.content
 
-        # PR worked
-        if r.status_code == 201:
-            log.debug("GitHub API PR worked:\n{}".format(returned_data_prettyprint))
-            log.info("GitHub PR created: {}".format(self.gh_pr_returned_data["html_url"]))
+            # PR worked
+            if r.status_code == 201:
+                log.debug("GitHub API PR worked:\n{}".format(returned_data_prettyprint))
+                log.info("GitHub PR created: {}".format(self.gh_pr_returned_data["html_url"]))
 
-        # Something went wrong
+            # Something went wrong
+            else:
+                raise PullRequestException(
+                    "GitHub API returned code {}: \n{}".format(r.status_code, returned_data_prettyprint)
+                )
         else:
-            raise PullRequestException(
-                "GitHub API returned code {}: \n{}".format(r.status_code, returned_data_prettyprint)
-            )
+            raise PullRequestException("Environment variable GITHUB_AUTH_TOKEN not set - cannot make PR")
 
     def reset_target_dir(self):
         """

tests/test_sync.py

@@ -191,7 +191,7 @@ def __init__(self, data, status_code):
                 self.status_code = status_code
                 self.content = json.dumps(data)
 
-        url_template = "https://api.github.com/repos/{}/response/pulls?head=nf-core:TEMPLATE&base=None"
+        url_template = "https://api.github.com/repos/{}/response/pulls?head=TEMPLATE&base=None"
         if kwargs["url"] == url_template.format("no_existing_pr"):
             response_data = []
             return MockResponse(response_data, 200)
@@ -258,9 +258,9 @@ def test_make_pull_request_bad_response(self, mock_get, mock_post):
     @mock.patch("requests.get", side_effect=mocked_requests_get)
     @mock.patch("requests.patch", side_effect=mocked_requests_patch)
     def test_update_existing_pull_request(self, mock_get, mock_patch):
-        """ Try discovering a PR and updating it """
+        """ Try closing a PR """
         psync = nf_core.sync.PipelineSync(self.pipeline_dir)
         psync.gh_username = "existing_pr"
         psync.gh_repo = "existing_pr/response"
         os.environ["GITHUB_AUTH_TOKEN"] = "test"
-        assert psync.update_existing_pull_request("title", "body") is True
+        assert psync.close_open_pr("TEMPLATE") is True