diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 619e99f8b72..48bf357c6f2 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -152,7 +152,7 @@ tiers partition the list and that the `pr` tier is exactly the default profile. | `pr_benchmark_check` | merge group, **or** PR with `run-benchmark-check` | benchmark sources only | | `delta_build_gate` | merge group, **or** PR with `run-delta-build-gate` | main sources, poms, `contrib/delta` | | `pyarrow_udf_test` | merge group, **or** PR with `run-pyarrow-udf-tests` | map-in-batch and Python runner code | -| `docs` | push to main, paths matched | `.asf.yaml`, `docs/**`, `docs.yaml` | +| `docs` | push to main, paths matched, **or** dispatch on `main` | `.asf.yaml`, `docs/**`, `docs.yaml` | | `spark_3_5` | nightly, **or** PR with `run-spark-3.5-tests` | Spark 3.5 sources | | `spark_4_1` | merge group, **or** PR with `run-spark-4.1-tests`; the `sql_hive` shards alone with `run-spark-4.1-hive-tests` | Spark 4.1 sources | | `spark_3_4` | PR with `run-spark-3.4-tests`, or dispatch | Spark 3.4 sources | @@ -169,6 +169,15 @@ its path filter or event criteria don't match. Skipped checks count as passing for branch protection, so a name that can report `skipped` is not safe to make a required check. +A pull request whose base is a release branch (`branch-N.M`) is the one +exception to the table: it runs every job the table puts in the PR, merge +group or nightly tier, because a release branch has no merge queue and no +nightly to run the last two later. `docs` stays push-only and `spark_3_4` +still needs its label, so a `labeled` run there adds `spark_3_4` or nothing. +`changes` hands the base branch to `compute-changes.py` as `PR_BASE_REF`. +`check-ci-config.py` pins that wiring, because a dropped variable reads as an +empty string and would route the pull request as if it targeted `main`. + ### Label events `ci_label.yml` fires on `pull_request.types: [labeled]` and calls `ci.yml` diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7f87d591995..8cf28c72dd2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,6 +40,11 @@ # Spark 3.4 is deprecated and sits outside all three tiers: it runs only when # a pull request carries `run-spark-3.4-tests`, or from a manual dispatch. # +# Release branches (`branch-N.M`) get neither the queue nor the nightly: the +# queue ruleset covers only the default branch, `push` below is main-only, and +# GitHub fires `schedule` only on the default branch. So a pull request that +# targets one runs all three tiers at once, still routed by the path filters. +# # Which tier a job sits in is POLICY in dev/ci/compute-changes.py, not an # expression here. Heavy jobs deliberately have no `push` tier: the queue # already tested the exact commit that lands, so re-running the pipeline on @@ -262,6 +267,7 @@ jobs: EVENT_ACTION: ${{ github.event.action }} LABEL_NAME: ${{ github.event.label.name }} PR_LABELS: ${{ toJSON(github.event.pull_request.labels.*.name) }} + PR_BASE_REF: ${{ github.event.pull_request.base.ref }} PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} MQ_BASE_SHA: ${{ github.event.merge_group.base_sha }} @@ -342,6 +348,7 @@ jobs: # # pull request linux, full -> profiles: pr # ... with the label linux, full, all -> profiles: all + # ... to branch-N.M linux, full, all -> profiles: all # `labeled` run all -> profiles: nightly # merge queue linux, full -> profiles: pr # nightly all -> profiles: nightly @@ -397,8 +404,11 @@ jobs: docs: name: Deploy Comet site needs: changes - # docs deploys to asf-site, so only run on push-to-main (or a manual dispatch). - if: needs.changes.outputs.docs == 'true' + # docs deploys to asf-site, so only run on push-to-main (or a manual + # dispatch of main). The ref check is for dispatch: it runs every job, and + # a dispatch on a release branch would otherwise publish that branch's docs + # over the site. + if: needs.changes.outputs.docs == 'true' && github.ref == 'refs/heads/main' uses: ./.github/workflows/docs.yaml spark_3_4: diff --git a/dev/ci/check-ci-config.py b/dev/ci/check-ci-config.py index 31b69675b7a..ecda4cf1b43 100644 --- a/dev/ci/check-ci-config.py +++ b/dev/ci/check-ci-config.py @@ -27,6 +27,13 @@ # could not be tested; POLICY_CASES below is the test it never had. The # expected sets are transcribed from the `if:` expressions ci.yml carried # before the policy moved, so a regression here is a behaviour change. +# The event reaches the script as environment variables set by the +# `Compute outputs` step of ci.yml's `Detect changes` job, and a variable +# dropped there reads as an empty string, which is a valid value (no +# label, no base branch), so that wiring is pinned as well. So is the +# `refs/heads/main` guard on the `docs` job: a dispatch routes every job, +# docs included, and that guard is all that keeps a dispatch on a release +# branch from publishing that branch's docs over the site. # # 3. Required-check coverage. `Required Checks` in ci.yml is the job that # `.asf.yaml` can name in `required_status_checks` for main. A heavy job @@ -390,6 +397,62 @@ }, set(), ), + # A pull request against a release branch runs the queue and nightly tiers + # as well, because a release branch has no queue and no nightly to run them + # later. The site deploy still never runs from a pull request, and the + # deprecated Spark 3.4 suite still waits for its label. + ( + {"name": "pull_request", "action": "opened", "labels": [], "base": "branch-1.1"}, + QUEUE_TIER | NIGHTLY_TIER, + ), + ( + {"name": "pull_request", "action": "synchronize", "labels": [], "base": "branch-2.0"}, + QUEUE_TIER | NIGHTLY_TIER, + ), + ( + { + "name": "pull_request", + "action": "synchronize", + "labels": ["run-spark-3.4-tests"], + "base": "branch-1.1", + }, + QUEUE_TIER | NIGHTLY_TIER | SPARK_DEPRECATED, + ), + # There the commit run already covered everything a label gates except + # Spark 3.4, so a `labeled` run adds Spark 3.4 or nothing. + ( + { + "name": "pull_request", + "action": "labeled", + "label": "run-spark-3.4-tests", + "labels": ["run-spark-3.4-tests"], + "base": "branch-1.1", + }, + SPARK_DEPRECATED, + ), + ( + { + "name": "pull_request", + "action": "labeled", + "label": "run-iceberg-tests", + "labels": ["run-iceberg-tests"], + "base": "branch-1.1", + }, + set(), + ), + # Only `branch-N.M` is a release branch. A pull request against main, or + # one stacked on another branch in the repository, keeps the PR tier. + ({"name": "pull_request", "action": "synchronize", "labels": [], "base": "main"}, PR_TIER), + ({"name": "pull_request", "action": "synchronize", "labels": [], "base": "pr-5654"}, PR_TIER), + ( + { + "name": "pull_request", + "action": "synchronize", + "labels": [], + "base": "branch-1.1-backports", + }, + PR_TIER, + ), ] @@ -654,6 +717,8 @@ def check_event_policy(): label += f"/{event['action']}" if event.get("label"): label += f" +{event['label']}" + if event.get("base"): + label += f" base={event['base']}" failures.append( f"{label} labels={event.get('labels', [])}: " f"unexpectedly allowed {sorted(actual - expected) or 'nothing'}, " @@ -688,6 +753,98 @@ def check_event_policy(): return not failures +# The environment variables `event_from_env` in compute-changes.py reads, and +# the `github` context expression ci.yml's `Compute outputs` step must set each +# one from. A typo in an expression is as silent as a missing variable: both +# arrive as an empty string. +EVENT_ENV_SOURCES = { + "EVENT_NAME": "github.event_name", + "EVENT_ACTION": "github.event.action", + "LABEL_NAME": "github.event.label.name", + "PR_LABELS": "toJSON(github.event.pull_request.labels.*.name)", + "PR_BASE_REF": "github.event.pull_request.base.ref", +} +ENV_READ = re.compile(r'os\.environ\.get\("([A-Z_]+)"') +ENV_SET = re.compile(r"^\s+([A-Z_]+):\s+\$\{\{\s*(.+?)\s*\}\}\s*$") + + +def compute_step_env(): + """{variable: expression} from the `env:` of ci.yml's `Compute outputs` step.""" + env, in_step, in_env = {}, False, False + for line in CI_WORKFLOW.read_text(encoding="utf-8").splitlines(): + if re.match(r"^\s+- name: Compute outputs\s*$", line): + in_step = True + elif in_step and not in_env: + if re.match(r"^\s+- ", line): + break # the next step; this one has no `env:` + in_env = bool(re.match(r"^\s+env:\s*$", line)) + elif in_env: + match = ENV_SET.match(line) + if not match: + break + env[match.group(1)] = match.group(2) + return env + + +def check_event_env(): + """Every variable POLICY reads is set by `Detect changes`, from the right context. + + Without PR_BASE_REF, for instance, every pull request against a release + branch would quietly fall back to the PR tier and every check would pass. + """ + failures = [] + source = Path("dev/ci/compute-changes.py").read_text(encoding="utf-8") + body = source.split("\ndef event_from_env", 1)[1].split("\ndef ", 1)[0] + read = set(ENV_READ.findall(body)) + env = compute_step_env() + for name in sorted(read - set(EVENT_ENV_SOURCES)): + failures.append( + f"event_from_env reads {name}, which EVENT_ENV_SOURCES does not " + f"list; add the `github` context expression it comes from" + ) + for name in sorted(set(EVENT_ENV_SOURCES) - read): + failures.append(f"EVENT_ENV_SOURCES lists {name}, which event_from_env no longer reads") + for name in sorted(read & set(EVENT_ENV_SOURCES)): + expected = EVENT_ENV_SOURCES[name] + if name not in env: + failures.append( + f"{CI_WORKFLOW}: the `Compute outputs` step does not set {name}, " + f"so compute-changes.py reads it as empty" + ) + elif env[name] != expected: + failures.append( + f"{CI_WORKFLOW}: the `Compute outputs` step sets {name} from " + f"`{env[name]}`, not `{expected}`" + ) + for failure in failures: + print(f"event env: {failure}") + return not failures + + +# The site deploy's job-level `if:` in ci.yml. It has to be on the job rather +# than in POLICY, because a dispatch routes every job (see POLICY_CASES). +DOCS_JOB = "docs" +DOCS_MAIN_GUARD = re.compile(r"^ if:.*github\.ref\s*==\s*'refs/heads/main'") + + +def check_docs_deploy_guard(): + """The site deploy runs from main only, whatever the event. + + docs.yaml rsyncs the built site over asf-site with --delete and falls back + to `git push --force`, and the release process dispatches ci.yml on the + release branch before every release candidate. + """ + _, guarded = guarded_jobs(CI_WORKFLOW, DOCS_MAIN_GUARD) + if DOCS_JOB in guarded: + return True + print( + f"docs deploy: the `{DOCS_JOB}` job in {CI_WORKFLOW} must require " + f"github.ref == 'refs/heads/main' in its `if:`, or a dispatch on a " + f"release branch publishes that branch's docs over the site" + ) + return False + + def artifact_names(path): """Return ([upload names], [download names]) for one workflow file.""" uploads, downloads = [], [] @@ -1277,6 +1434,8 @@ def check_cache_save_scope(): if __name__ == "__main__": ok = check_change_filters() ok = check_event_policy() and ok + ok = check_event_env() and ok + ok = check_docs_deploy_guard() and ok ok = check_spark_sql_modules() and ok ok = check_linux_test_profiles() and ok ok = check_artifact_names() and ok diff --git a/dev/ci/compute-changes.py b/dev/ci/compute-changes.py index ae29077db4c..7ab4eb0ec21 100644 --- a/dev/ci/compute-changes.py +++ b/dev/ci/compute-changes.py @@ -435,6 +435,17 @@ # Adding "push" back to a test job would make every merge run it twice, once # in the queue and once after, which is the thing the queue was adopted to # avoid. +# +# A pull request that targets a release branch (`branch-N.M`) is the one +# exception: it runs every "pr", "queue" and "nightly" job. On main the tiers +# spread the suites over three events, and every change still meets all of +# them. A release branch has only the pull request: the merge queue covers the +# default branch alone, ci.yml runs on push only for main, and GitHub fires +# `schedule` only on the default branch. A tier there would not defer a suite, +# it would drop it. And the tiers exist because of main's volume, which a +# release branch does not have: branch-1.0 took 28 pull requests in its first +# two months. "push" jobs (the site deploy) and label-only jobs (Spark 3.4) +# keep their rules on a release branch. POLICY = { # The one test job that also runs on push to main, and only because of # actions/cache scoping: a pull request can restore caches saved on its @@ -518,6 +529,13 @@ } +# Release branches are named `branch-.`, as in branch-0.17 and +# branch-1.0. A pull request against one runs these tiers; see the end of the +# comment above POLICY. +RELEASE_BRANCH = re.compile(r"branch-\d+\.\d+") +RELEASE_BRANCH_TIERS = ("pr", "queue", "nightly") + + def gating_labels(job): return [t[len("label:"):] for t in POLICY[job] if t.startswith("label:")] @@ -525,9 +543,9 @@ def gating_labels(job): def event_allows(job, event): """Does `event` permit `job` to run, ignoring which files changed? - `event` is {"name", "action", "label", "labels"}: the workflow event name, - the pull_request action, the label just added on a `labeled` event, and the - labels currently on the pull request. + `event` is {"name", "action", "label", "labels", "base"}: the workflow event + name, the pull_request action, the label just added on a `labeled` event, + the labels currently on the pull request, and the branch it targets. """ tiers = POLICY[job] name = event.get("name") @@ -542,6 +560,8 @@ def event_allows(job, event): return "nightly" in tiers if name != "pull_request": return False + if RELEASE_BRANCH.fullmatch(event.get("base", "")): + return release_branch_allows(job, event) gates = gating_labels(job) if gates: @@ -560,6 +580,19 @@ def event_allows(job, event): return True +def release_branch_allows(job, event): + """event_allows for a pull request that targets a release branch.""" + tiers = POLICY[job] + gates = gating_labels(job) + automatic = any(tier in tiers for tier in RELEASE_BRANCH_TIERS) + # The opened/synchronize run already ran every automatic job at this + # commit, so a `labeled` run adds only a job that nothing else runs there, + # which leaves the label-only Spark 3.4 suite. + if event.get("action") == "labeled": + return not automatic and event.get("label") in gates + return automatic or any(label in event.get("labels", []) for label in gates) + + def compute(files, event): """Return {job: bool}, folding the path filter and the event policy.""" return { @@ -575,6 +608,7 @@ def event_from_env(): "action": os.environ.get("EVENT_ACTION", ""), "label": os.environ.get("LABEL_NAME", ""), "labels": json.loads(labels) if labels.strip() else [], + "base": os.environ.get("PR_BASE_REF", ""), } diff --git a/docs/source/contributor-guide/ci.md b/docs/source/contributor-guide/ci.md index fa367de798a..57f27fe96e7 100644 --- a/docs/source/contributor-guide/ci.md +++ b/docs/source/contributor-guide/ci.md @@ -105,6 +105,9 @@ Which tier a job belongs to is the `POLICY` table in `dev/ci/compute-changes.py` filters are the `FILTERS` table in the same file, and `dev/ci/check-ci-config.py` holds the test cases that pin both down. +A pull request against a release branch runs all three tiers at once; see +[Release branches](#release-branches). + ## Opting a pull request into a suite the PR tier skips Each suite outside the PR tier has a label that runs it on a pull request: @@ -341,6 +344,41 @@ and Iceberg 1.11. If everything is skipped, open the run's `Detect changes` job: `Nightly base:` commit it diffed against and the list of changed files, which is enough to tell a genuinely quiet day from a base that has drifted. +## Release branches + +Release branches (`branch-N.M`) start with the workflow files `main` had when they were cut, but have +no merge queue and no nightly run. The merge queue covers only `main`, `ci.yml` runs on push only for `main`, and GitHub +fires scheduled workflows only on the default branch, so a release branch gets no scheduled `ci.yml`, +Miri or CodeQL run, and nothing runs after a pull request merges. + +On `main`, the tiers hold the expensive suites back for the queue and the nightly run, which still +test every change. A release branch has neither, so holding a suite back there would mean never +running it. A pull request that targets a release branch, such as a backport, therefore runs the PR, +queue and nightly tiers together. Release branches get few pull requests, so this costs little. The +path filters still apply, so a documentation-only change runs none of the heavy suites. The Spark +SQL suite for Spark 3.4 still needs its `run-spark-3.4-tests` label, and the other `run-*` labels +have nothing to add there. + +A release branch runs the workflow files committed on it, so a change to these rules on `main` +reaches a release branch only if it is backported. `branch-1.0` predates the tiers and follows its +own, older rules. + +Each pull request is tested against the release branch as it was when its run started, and with no +merge queue, nothing tests the result of merging it. Two backports that pass on their own can still +break the branch once both have landed. To run every suite against a release branch as it stands, +dispatch `ci.yml` on it. A dispatch runs every job in that branch's own `ci.yml`, including `docs`, +which publishes the website. So first check that the `if:` of the branch's `docs` job requires +`github.ref == 'refs/heads/main'`. A branch without that guard publishes its own docs over the site. + +```sh +git show apache/branch-N.M:.github/workflows/ci.yml | sed -n '/^ docs:/,/uses:/p' +gh workflow run ci.yml --repo apache/datafusion-comet --ref branch-N.M +``` + +The release process does this before tagging each release candidate; see +[Run the Full CI Suite](release_process.md#run-the-full-ci-suite). A failed dispatched run opens no +`ci-nightly-failure` issue. + ## Reproducing a suite failure locally `dev/local-ci.sh` builds the same sandbox a runner builds and runs the Spark SQL or Iceberg diff --git a/docs/source/contributor-guide/release_process.md b/docs/source/contributor-guide/release_process.md index 842ace9dcdb..4555d6780ca 100644 --- a/docs/source/contributor-guide/release_process.md +++ b/docs/source/contributor-guide/release_process.md @@ -36,6 +36,7 @@ instructions on each step. - [ ] Update Maven version in release branch - [ ] Update version in main for next development cycle - [ ] Generate the change log and PR it against the release branch +- [ ] Run the full CI suite on the release branch - [ ] Build the jars - [ ] Tag the release candidate - [ ] Update documentation for the new release @@ -133,6 +134,22 @@ protected_branches: All release branches stay protected, including older ones, so released code cannot be pushed to directly. +Once the pull request merges, check that the protection took effect: + +```shell +gh api repos/apache/datafusion-comet/branches/branch-0.13 --jq .protected +``` + +This prints `true` once ASF has applied the change. If it still prints `false`, check that the branch is listed +under `protected_branches` on `main`. + +Protection requires a review but not a green CI run, so check that a pull request's run passed before merging it. +The release branch has no merge queue, no nightly run, and no CI on push, so a pull request targeting it runs every +suite that the PR, queue, and nightly tiers run on `main`. That includes the version bump pull request below and +every backport. The documentation and change log pull requests change only Markdown, so the path filters skip the +heavy suites for them. [Release branches](ci.md#release-branches) explains what runs and +why. The [full CI run before tagging](#run-the-full-ci-suite) tests the branch with every change merged. + ### Generate Release Documentation The docs on `main` contain only template markers; CI fills them at publish time. A release branch instead @@ -170,8 +187,10 @@ Any hit outside the change log is a place that will break on the release branch. `-SNAPSHOT` qualifier changes the built artifact file names, so anything that locates the jar by glob must match a bare version too. Prefer patterns such as `comet-spark-spark3.5_2.12-*.jar` over `comet-spark-spark3.5_2.12-*-SNAPSHOT.jar`, and prefer resolving the jar by glob over hardcoding a version. -The release branch runs the same CI workflows as `main`, including the PyArrow UDF tests, so a -`-SNAPSHOT`-only glob in a test harness or script fails only after the release branch is cut. +The release branch has the same CI workflows as `main`, so a `-SNAPSHOT`-only glob in a test harness or script +fails only after the release branch is cut. The version bump pull request is where it shows up. It changes the +`pom.xml` files, so it runs the suites that find the jar by file name or version, such as the PyArrow UDF tests and +the Spark SQL and Iceberg suites. The Spark SQL suite for Spark 3.4 runs only with the `run-spark-3.4-tests` label. ### Update Version in main @@ -209,6 +228,46 @@ branch are complete; if more changes land on the release branch before the relea regenerate it and update the PR. After the release is approved and tagged, open a separate PR to bring the same change log file into `main`. +### Run the Full CI Suite + +Once the generated docs, version bump, and change log have merged to the release branch, run every CI suite +against it. Each pull request ran against the branch as it stood when its run started, so nothing has yet tested +the branch with all of them merged. Pull requests there also skip the Spark SQL suite for Spark 3.4 unless it is +labeled, and Miri, which runs only on a schedule on `main`. + +A dispatch runs every job in the release branch's own `ci.yml`, including `docs`, which publishes the website. So +first check that the branch limits that job to `main`: the `if:` this prints must require +`github.ref == 'refs/heads/main'`. A branch without that guard publishes its own docs over the site. + +```shell +git fetch apache +git show apache/branch-0.13:.github/workflows/ci.yml | sed -n '/^ docs:/,/uses:/p' +``` + +Then dispatch both workflows on the release branch: + +```shell +gh workflow run ci.yml --repo apache/datafusion-comet --ref branch-0.13 +gh workflow run miri.yml --repo apache/datafusion-comet --ref branch-0.13 +``` + +A dispatched `ci.yml` run ignores the tiers and the path filters. It runs every suite in the +[tier table](ci.md#three-tiers), including the Spark SQL suite for Spark 3.4, which sits outside every tier. +`miri.yml` runs the unsafe code checks, which are not part of `ci.yml`. Expect the runs to take a few hours. + +A failed dispatched run does not open a `ci-nightly-failure` issue, so check the result yourself. This prints the +latest dispatched run of each workflow on the branch: + +```shell +for wf in ci.yml miri.yml; do + gh api "repos/apache/datafusion-comet/actions/workflows/$wf/runs?branch=branch-0.13&event=workflow_dispatch&per_page=1" \ + --jq ".workflow_runs[] | \"$wf\t\(.head_sha)\t\(.status)\t\(.conclusion)\t\(.html_url)\"" +done +``` + +Both runs must be green at the commit you are about to tag. If anything merges to the release branch after the +runs start, run them again. Repeat this for every release candidate. + ### Build the jars #### A note on workspace cleanliness @@ -275,7 +334,8 @@ repository ### Tag the Release Candidate -Ensure that the Maven version update and change log have been merged to the release branch before tagging. +Ensure that the Maven version update and change log have been merged to the release branch, and that the +[full CI run](#run-the-full-ci-suite) is green at the commit you are tagging, before tagging. Tag the release branch with `0.13.0-rc1` and push to the `apache` repo