Allow other refspecs to be used.

That gives us a way to test e.g. pull requests by specifying
ph_scripts_refspec="pull/123/head"

Also keep "ph_" prefix to remove confusion we had between
"scripts_branch" and "ph_scripts_branch" before.

Update docs and moved "testing" sections up.

docs/development.md

@@ -43,23 +43,28 @@ account for writing comments.
 
 Buildkite allows [dynamically define pipelines as the output of a
 command](https://buildkite.com/docs/pipelines/defining-steps#dynamic-pipelines).
-That gives us the flexibility to generate pipeline code using a script from a
+That gives us the flexibility to generate pipeline code using the code from a
-specific branch of pre-merge checks.
+specific branch of pre-merge checks. Thus, 
+[changes can be tested](./playbooks.md#testing-changes-before-merging)
+before affecting everyone.
 
-For example, "pre-merge" pipeline has a single fixed step, that checks out this
+For example, "pre-merge" pipeline has a single "setup" step, that checks out this
 repo and runs a python script to generate further steps:
 
 ```shell script
-export SRC=${BUILDKITE_BUILD_PATH}/llvm-premerge-checks
+export SRC="${BUILDKITE_BUILD_PATH}"/llvm-premerge-checks
-rm -rf ${SRC}
+export SCRIPT_DIR="${SRC}"/scripts
-git clone --depth 1 --branch ${scripts_branch:-master} https://github.com/google/llvm-premerge-checks.git ${SRC}
+rm -rf "${SRC}"
-export SCRIPT_DIR=${SRC}/scripts
+git clone --depth 1 https://github.com/google/llvm-premerge-checks.git "${SRC}"
+cd "${SRC}"
+git fetch origin "${ph_scripts_refspec:-master}":x
+git checkout x
+cd "$BUILDKITE_BUILD_CHECKOUT_PATH"
 ${SCRIPT_DIR}/buildkite/build_branch_pipeline.py | tee /dev/tty | buildkite-agent pipeline upload
 ```
 
-One typically edits corresponding script, not a pipeline definition in the
+One typically edits corresponding script, instead of manually updating a pipeline
-Buildkite interface. [How to test
+in the Buildkite interface.
-changes](playbooks.md#testing-changes-before-merging).
 
 # Life of a pre-merge check
 
@@ -69,8 +74,8 @@ testers").
 That in sends an HTTP POST request to [**phab-proxy**](../phabricator-proxy)
 that submits a new buildkite job **diff-checks**. All parameters from the
 original request are put in the build's environment with `ph_` prefix (to avoid
-shadowing any Buildkite environment variable). "scripts_branch" parameter
+shadowing any Buildkite environment variable). "ph_scripts_refspec" parameter
-defines which branch of llvm-premerge-checks to use.
+defines refspec of llvm-premerge-checks to use ("master" by default).
 
 **diff-checks** pipeline
 ([create_branch_pipeline.py](../scripts/buildkite/create_branch_pipeline.py))

docs/playbooks.md

@@ -1,16 +1,41 @@
 - [Playbooks](#playbooks)
-  * [deployment to a clean infrastructure](#deployment-to-a-clean-infrastructure)
+  * [Testing scripts locally](#testing-scripts-locally)
+  * [Testing changes before merging](#testing-changes-before-merging)
+  * [Deployment to a clean infrastructure](#deployment-to-a-clean-infrastructure)
   * [Creating docker containers on Windows](#creating-docker-containers-on-windows)
   * [Spawning a new windows agent](#spawning-a-new-windows-agent)
     + [Buildkite](#buildkite)
-  * [Testing scripts locally](#testing-scripts-locally)
   * [Custom environment variables](#custom-environment-variables)
-  * [Testing changes before merging](#testing-changes-before-merging)
   * [Update HTTP auth credentials](#update-http-auth-credentials)
   
 # Playbooks
 
-## deployment to a clean infrastructure
+## Testing scripts locally
+
+Build and run agent docker image `sudo ./containers/build_run.sh buildkite-premerge-debian /bin/bash`.
+
+Set `CONDUIT_TOKEN` with your personal one from `https://reviews.llvm.org/settings/user/<USERNAME>/page/apitokens/`.
+
+## Testing changes before merging
+
+It's recommended to test even smallest changes before committing them to the `master` branch.
+
+1. Create a pull request here.
+1. Manually create a buildkite build in the pipeline you are updating and specify
+    environment variable `ph_scripts_refspec="pull/123/head"`. Replace `123` 
+    with your PR number. If you don't have access to create buildkite builds, 
+    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).   
+   
+   See also [custom environment variables](#custom-environment-variables).
+1. Wait for build to complete and maybe attach a link to it to your PR.
+
+To test changes for the pipeline "setup" step please experiment on a copy first. 
+
+## Deployment to a clean infrastructure
 
 General remarks:
 * GCP does not route any traffic to your services unless the service is
@@ -131,12 +156,6 @@ git clone https://github.com/google/llvm-premerge-checks.git C:\llvm-premerge-ch
 schtasks.exe /create /tn "Start Buildkite agent" /ru SYSTEM /SC ONSTART /DELAY 0005:00 /tr "powershell -command 'C:\llvm-premerge-checks\scripts\windows_agent_start_buildkite.ps1 -workdir c:\ws'"
 ```
 
-## Testing scripts locally
-
-Build and run agent docker image `sudo ./containers/build_run.sh buildkite-premerge-debian /bin/bash`.
-
-Additionally set `WORKSPACE`, `PHID` and `DIFF_ID` parameters. Set `CONDUIT_TOKEN` with your personal one from `https://reviews.llvm.org/settings/user/<USERNAME>/page/apitokens/`.
-
 ## Custom environment variables
 
 Buildkite pipelines have a number of custom environment variables one can set to change their behavior. That is useful to debug issues
@@ -145,7 +164,7 @@ please refer to the source code for the details. These variables have `ph_` pref
 
 Most commonly used are:
 
-- `scripts_branch` ("master" by default): which branch of llvm-premerge-checks to use. This variable is also used in pipeline "bootstrap" in Buildkite interface.
+- `ph_scripts_refspec` ("master" by default): refspec branch of llvm-premerge-checks to use. This variable is also used in pipeline "bootstrap" in Buildkite interface.
 - `ph_no_cache`: (if set to any value) clear compilation cache before the build.
 - `ph_projects`: which projects to use, "detect" will look on diff to infer the projects, "default" selects all projects.
 - `ph_notify_email`: comma-separated list of email addresses to be notified when build is complete.
@@ -154,15 +173,6 @@ Most commonly used are:
 - `ph_linux_agents`, `ph_windows_agents`: custom JSON constraints on agents. For example you might put one machine to a custom queue if it's errornous and send jobs to it with `ph_windows_agents="{{\"queue\": \"custom\"}}"`.
 - `ph_skip_linux`, `ph_skip_windows` (if set to any value): skip build on this OS.
 
-## Testing changes before merging
-
-It's recommended to test even smallest changes before committing them to the `master` branch.
-
-1. Create a branch with your changes, e.g. "my-feature" and push it to origin.
-1. Manually create a buildkite build in the pipeline you are updating and specify environment variable
-   `scripts_branch="my-feature"` (see also "Custom environment variables" for other options above). To test "premerge-tests" pipeline pick an existing build and copy parameters from it, omitting "ph_target_phid", namely: "ph_build_id", "ph_buildable_diff", "ph_buildable_revision", "ph_initiator_phid" and "scripts_branch" variables.
-1. Wait for build to complete and maybe attach a link to it to your PR.
-
 ## Update HTTP auth credentials
 
 To update e.g. buildkite http-auth:

phabricator-proxy/main.py

@@ -27,12 +27,12 @@ def build():
         for k, v in params.items():
             if len(v) == 1:
                 build_env['ph_' + k] = v[0]
-        branch = 'master'
+        refspec = 'master'
-        if 'ph_scripts_branch' in build_env:
+        if 'ph_scripts_refspec' in build_env:
-            branch = build_env['ph_scripts_branch']
+            refspec = build_env['ph_scripts_refspec']
         build_request = {
             'commit': 'HEAD',
-            'branch':  branch,
+            'branch':  refspec,
             'env': build_env,
             'message': f'D{build_env["ph_buildable_revision"]}',
         }

scripts/buildkite/build_branch_pipeline.py

@@ -18,13 +18,14 @@ import os
 import yaml
 
 if __name__ == '__main__':
-    scripts_branch = os.getenv("scripts_branch", "master")
+    scripts_refspec = os.getenv("ph_scripts_refspec", "master")
     queue_prefix = os.getenv("ph_queue_prefix", "")
     diff_id = os.getenv("ph_buildable_diff", "")
     no_cache = os.getenv('ph_no_cache') is not None
     filter_output = '--filter-output' if os.getenv('ph_no_filter_output') is None else ''
     projects = os.getenv('ph_projects', 'detect')
     log_level = os.getenv('ph_log_level', 'WARNING')
+
     steps = []
     linux_agents = {'queue': f'{queue_prefix}linux'}
     t = os.getenv('ph_linux_agents')
@@ -40,12 +41,17 @@ if __name__ == '__main__':
             'ccache --show-config',
             'mkdir -p artifacts',
             'dpkg -l >> artifacts/packages.txt',
+            # Clone scripts.
             'export SRC=${BUILDKITE_BUILD_PATH}/llvm-premerge-checks',
             'rm -rf ${SRC}',
-            f'git clone --depth 1 --branch {scripts_branch} https://github.com/google/llvm-premerge-checks.git '
+            'git clone --depth 1 https://github.com/google/llvm-premerge-checks.git "${SRC}"',
-            '${SRC}',
+            'cd ${SRC}',
+            f'git fetch origin "{scripts_refspec}":x',
+            'git checkout x',
             'echo "llvm-premerge-checks commit"',
-            'git --git-dir ${SRC}/.git rev-parse HEAD',
+            'git rev-parse HEAD',
+            'cd "$BUILDKITE_BUILD_CHECKOUT_PATH"',
+
             'set +e',
             # Add link in review to the build.
             '${SRC}/scripts/phabtalk/add_url_artifact.py '
@@ -81,11 +87,18 @@ if __name__ == '__main__':
         'commands': [
             clear_sccache if no_cache else '',
             'sccache --zero-stats',
+
+            # Clone scripts.
             'set SRC=%BUILDKITE_BUILD_PATH%/llvm-premerge-checks',
             'rm -rf %SRC%',
-            f'git clone --depth 1 --branch {scripts_branch} https://github.com/google/llvm-premerge-checks.git %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 --git-dir %SRC%/.git rev-parse HEAD',
+            'git rev-parse HEAD',
+            'cd %BUILDKITE_BUILD_CHECKOUT_PATH%',
+
             'powershell -command "'
             f'%SRC%/scripts/premerge_checks.py --projects=\'{projects}\' --log-level={log_level} {filter_output}; '
             '\\$exit=\\$?;'
@@ -122,7 +135,7 @@ if __name__ == '__main__':
             'buildkite-agent artifact download "*_result.json" .',
             'export SRC=${BUILDKITE_BUILD_PATH}/llvm-premerge-checks',
             'rm -rf ${SRC}',
-            f'git clone --depth 1 --branch {scripts_branch} https://github.com/google/llvm-premerge-checks.git '
+            f'git clone --depth 1 --branch {scripts_refspec} https://github.com/google/llvm-premerge-checks.git '
             '${SRC}',
             '${SRC}/scripts/buildkite/summary.py',
         ],

scripts/buildkite/build_master_pipeline.py

@@ -18,7 +18,7 @@ import os
 import yaml
 
 if __name__ == '__main__':
-    scripts_branch = os.getenv("scripts_branch", "master")
+    scripts_refspec = os.getenv("ph_scripts_refspec", "master")
     queue_prefix = os.getenv("ph_queue_prefix", "")
     no_cache = os.getenv('ph_no_cache') is not None
     filter_output = '--filter-output' if os.getenv('ph_no_filter_output') is None else ''
@@ -40,12 +40,17 @@ if __name__ == '__main__':
             'ccache --show-config',
             'mkdir -p artifacts',
             'dpkg -l >> artifacts/packages.txt',
+            # Clone scripts.
             'export SRC=${BUILDKITE_BUILD_PATH}/llvm-premerge-checks',
             'rm -rf ${SRC}',
-            f'git clone --depth 1 --branch {scripts_branch} https://github.com/google/llvm-premerge-checks.git '
+            'git clone --depth 1 https://github.com/google/llvm-premerge-checks.git "${SRC}"',
-            '${SRC}',
+            'cd ${SRC}',
+            f'git fetch origin "{scripts_refspec}":x',
+            'git checkout x',
             'echo "llvm-premerge-checks commit"',
-            'git --git-dir ${SRC}/.git rev-parse HEAD',
+            'git rev-parse HEAD',
+            'cd "$BUILDKITE_BUILD_CHECKOUT_PATH"',
+
             'set +e',
             f'${{SRC}}/scripts/premerge_checks.py --projects="{projects}" --log-level={log_level} {filter_output}',
             'EXIT_STATUS=\\$?',
@@ -77,11 +82,18 @@ if __name__ == '__main__':
         'commands': [
             clear_sccache if no_cache else '',
             'sccache --zero-stats',
+
+            # Clone scripts.
             'set SRC=%BUILDKITE_BUILD_PATH%/llvm-premerge-checks',
             'rm -rf %SRC%',
-            f'git clone --depth 1 --branch {scripts_branch} https://github.com/google/llvm-premerge-checks.git %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 --git-dir %SRC%/.git rev-parse HEAD',
+            'git rev-parse HEAD',
+            'cd %BUILDKITE_BUILD_CHECKOUT_PATH%',
+
             'powershell -command "'
             f'%SRC%/scripts/premerge_checks.py --projects=\'{projects}\' --log-level={log_level} {filter_output}; '
             '\\$exit=\\$?;'

scripts/buildkite/create_branch_pipeline.py

@@ -41,12 +41,15 @@ if __name__ == '__main__':
         'depends_on': 'create-branch',
         'build': {
             'branch': f'phab-diff-{diff_id}',
-            'env': {'scripts_branch': '${BUILDKITE_BRANCH}'},
+            'env': {},
         },
     }
     for e in os.environ:
         if e.startswith('ph_'):
             build_linux_step['build']['env'][e] = os.getenv(e)
+    # Set scripts source from the current build if it's not yet defined.
+    if 'ph_scripts_refspec' not in build_linux_step['build']['env']:
+        build_linux_step['build']['env']['ph_scripts_refspec'] = '${BUILDKITE_BRANCH}'
     steps.append(create_branch_step)
     if run_build:
         steps.append(build_linux_step)