Don't fail if phabricator target is not specified

That is useful when testing pipelines

docs/playbooks.md

@@ -27,8 +27,8 @@ It's recommended to test even smallest changes before committing them to the `ma
     please ask a reviewer to do that.
   
    To test "premerge-tests" pipeline pick an existing build and copy "ph_"
-   parameters from it, omitting "ph_target_phid" (so that build does not update 
-   existing review. That will likely lead to an error though).   
+   parameters from it, omitting "ph_target_phid" to skip updating an existing
+   review.   
    
    See also [custom environment variables](#custom-environment-variables).
 1. Wait for build to complete and maybe attach a link to it to your PR.

scripts/phabtalk/phabtalk.py

@@ -74,7 +74,7 @@ class PhabTalk:
                                               transactions=transactions)
         print('Uploaded comment to Revision D{}:{}'.format(revision, text))
 
-    def update_build_status(self, diff_id: str, phid: str, working: bool, success: bool, lint: {} = {}, unit: [] = []):
+    def update_build_status(self, phid: str, working: bool, success: bool, lint: {}, unit: []):
         """Submit collected report to Phabricator.
         """
 

scripts/pipeline_premerge.py

@@ -66,34 +66,35 @@ if __name__ == '__main__':
     else:
         logging.info('No projects for default checks')
 
-    deps = []
+    if os.getenv('ph_target_phid') is None:
+        logging.warning('ph_target_phid is not specified. Skipping "Report" step')
+    else:
+        steps.append({
+            'wait': '~',
+            'continue_on_failure': True,
+        })
 
-    steps.append({
-        'wait': '~',
-        'continue_on_failure': True,
-    })
+        report_step = {
+            'label': ':spiral_note_pad: report',
+            'commands': [
+                'mkdir -p artifacts',
+                'buildkite-agent artifact download "*_result.json" .',
 
-    report_step = {
-        'label': ':spiral_note_pad: report',
-        'commands': [
-            'mkdir -p artifacts',
-            'buildkite-agent artifact download "*_result.json" .',
-
-            # Clone scripts.
-            'export SRC=${BUILDKITE_BUILD_PATH}/llvm-premerge-checks',
-            'rm -rf ${SRC}',
-            'git clone --depth 1 https://github.com/google/llvm-premerge-checks.git "${SRC}"',
-            'cd ${SRC}',
-            f'git fetch origin "{scripts_refspec}":x',
-            'git checkout x',
-            'echo "llvm-premerge-checks commit"',
-            'git rev-parse HEAD',
-            'cd "$BUILDKITE_BUILD_CHECKOUT_PATH"',
-            '${SRC}/scripts/summary.py',
-        ],
-        'artifact_paths': ['artifacts/**/*'],
-        'agents': {'queue': 'linux'},
-        'timeout_in_minutes': 10,
-    }
-    steps.append(report_step)
+                # Clone scripts.
+                'export SRC=${BUILDKITE_BUILD_PATH}/llvm-premerge-checks',
+                'rm -rf ${SRC}',
+                'git clone --depth 1 https://github.com/google/llvm-premerge-checks.git "${SRC}"',
+                'cd ${SRC}',
+                f'git fetch origin "{scripts_refspec}":x',
+                'git checkout x',
+                'echo "llvm-premerge-checks commit"',
+                'git rev-parse HEAD',
+                'cd "$BUILDKITE_BUILD_CHECKOUT_PATH"',
+                '${SRC}/scripts/summary.py',
+            ],
+            'artifact_paths': ['artifacts/**/*'],
+            'agents': {'queue': 'linux'},
+            'timeout_in_minutes': 10,
+        }
+        steps.append(report_step)
     print(yaml.dump({'steps': steps}))

scripts/premerge_checks.py

@@ -170,18 +170,17 @@ if __name__ == '__main__':
         print('^^^ +++', flush=True)
 
     ph_target_phid = os.getenv('ph_target_phid')
-    ph_buildable_diff = os.getenv('ph_buildable_diff')
     if ph_target_phid is not None:
         phabtalk = PhabTalk(os.getenv('CONDUIT_TOKEN'))
         for u in report.unit:
             u['engine'] = step_key
-        phabtalk.update_build_status(ph_buildable_diff, ph_target_phid, True, report.success, report.lint, report.unit)
+        phabtalk.update_build_status(ph_target_phid, True, report.success, report.lint, report.unit)
         for a in report.artifacts:
             url = upload_file(a['dir'], a['file'])
             if url is not None:
                 phabtalk.maybe_add_url_artifact(ph_target_phid, url, f'{a["name"]} ({step_key})')
     else:
-        logging.warning('No phabricator phid is specified. Will not update the build status in Phabricator')
+        logging.warning('ph_target_phid is not specified. Will not update the build status in Phabricator')
     with open(report_path, 'w') as f:
         json.dump(report.__dict__, f, default=as_dict)
 

scripts/set_build_status.py

@@ -26,12 +26,16 @@ if __name__ == '__main__':
     parser.add_argument('--success', action='store_true')
     args = parser.parse_args()
     logging.basicConfig(level=args.log_level, format='%(levelname)-7s %(message)s')
+
     phabtalk = PhabTalk(os.getenv('CONDUIT_TOKEN'))
+    ph_target_phid = os.getenv('ph_target_phid')
+    if ph_target_phid is None:
+        logging.warning('ph_target_phid is not specified. Will not update the build status in Phabricator')
+        exit(0)
     build_url = f'https://reviews.llvm.org/harbormaster/build/{os.getenv("ph_build_id")}'
     print(f'Reporting results to Phabricator build {format_url(build_url)}', flush=True)
     ph_buildable_diff = os.getenv('ph_buildable_diff')
-    ph_target_phid = os.getenv('ph_target_phid')
-    phabtalk.update_build_status(ph_buildable_diff, ph_target_phid, False, args.success)
+    phabtalk.update_build_status(ph_target_phid, False, args.success, {}, [])
     bug_url = f'https://github.com/google/llvm-premerge-checks/issues/new?assignees=&labels=bug' \
               f'&template=bug_report.md&title=buildkite build {os.getenv("BUILDKITE_PIPELINE_SLUG")} ' \
               f'{os.getenv("BUILDKITE_BUILD_NUMBER")}'

scripts/summary.py

@@ -39,7 +39,10 @@ if __name__ == '__main__':
               f'{os.getenv("BUILDKITE_TRIGGERED_FROM_BUILD_PIPELINE_SLUG")}/' \
               f'builds/{os.getenv("BUILDKITE_TRIGGERED_FROM_BUILD_NUMBER")}'
         print(f'Triggered from build {format_url(url)}', flush=True)
-
+    ph_target_phid = os.getenv('ph_target_phid')
+    if ph_target_phid is None:
+        logging.warning('ph_target_phid is not specified. Will not update the build status in Phabricator')
+        exit(0)
     success = True
     for path in glob.glob("*_result.json"):
         logging.info(f'analysing {path}')
@@ -50,9 +53,7 @@ if __name__ == '__main__':
     phabtalk = PhabTalk(os.getenv('CONDUIT_TOKEN'))
     build_url = f'https://reviews.llvm.org/harbormaster/build/{os.getenv("ph_build_id")}'
     print(f'Reporting results to Phabricator build {format_url(build_url)}', flush=True)
-    ph_buildable_diff = os.getenv('ph_buildable_diff')
-    ph_target_phid = os.getenv('ph_target_phid')
-    phabtalk.update_build_status(ph_buildable_diff, ph_target_phid, False, success)
+    phabtalk.update_build_status(ph_target_phid, False, success, {}, [])
     bug_url = f'https://github.com/google/llvm-premerge-checks/issues/new?assignees=&labels=bug' \
               f'&template=bug_report.md&title=buildkite build {os.getenv("BUILDKITE_PIPELINE_SLUG")} ' \
               f'{os.getenv("BUILDKITE_BUILD_NUMBER")}'