You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by jo...@apache.org on 2026/09/28 14:10:34 UTC
svn commit: r1938616 - in httpd/dev-tools/trunk/skills: . backport pr-test
Author: jorton
Date: Mon Sep 28 14:10:34 2026
New Revision: 1938616
Log:
Add two Claude skills:
1) /backport, for creating a 2.4.x PR based off backports from
trunk. This can either be given SVN revs directly, or text input
which Claude will interpret, "/backport mod_systemd changes from
the last month", for example.
2) /pr-test which tries to create a pyhttpd-based test case for
an existing PR which doesn't include one already
Co-Authored-By: Claude Opus 5.5 <no...@anthropic.com>
Added:
httpd/dev-tools/trunk/skills/
httpd/dev-tools/trunk/skills/backport/
httpd/dev-tools/trunk/skills/backport/SKILL.md
httpd/dev-tools/trunk/skills/pr-test/
httpd/dev-tools/trunk/skills/pr-test/SKILL.md
Added: httpd/dev-tools/trunk/skills/backport/SKILL.md
==============================================================================
--- /dev/null 00:00:00 1970 (empty, because file is newly added)
+++ httpd/dev-tools/trunk/skills/backport/SKILL.md Mon Sep 28 14:10:34 2026 (r1938616)
@@ -0,0 +1,225 @@
+---
+name: backport
+description: Propose a backport of trunk revisions to 2.4.x - cherry-pick each rev into its own commit on a new branch and worktree off origin/2.4.x, build it, and open a "[2.4.x] Merge ..." PR after previewing the text, then draft its STATUS entry. Use for "backport r1234567 to 2.4.x", "propose a backport of <hash>", "/backport mod_ssl r1934975 r1934980".
+---
+
+# Propose a trunk → 2.4.x backport
+
+Given one or more trunk revisions, build a `2.4.x-<topic>` branch off `origin/2.4.x` with
+**one commit per trunk revision**, build it, and open a PR against `2.4.x` whose body
+quotes the trunk commit messages.
+
+Usage: `/backport <topic> r1234567 [r2345678 | <githash> ...]`. Revisions may be given as
+svn revs or trunk git hashes. `<topic>` names the branch and fills the PR subject (e.g.
+`mod_ssl`, `mod_proxy_http`); if it is not given, pick one from the files touched and say
+so.
+
+## 0. Preflight
+
+- If the checkout has an `AGENTS.md`, read it first and follow it. Local guidance there
+ (paths, build setup, commit and PR conventions) takes precedence over the defaults in
+ this skill.
+- `git fetch origin`. Base on **`origin/2.4.x`**; local `trunk`/`2.4.x` are stale.
+- Stay inside this checkout. The only outside paths touched are the read-only APR source
+ trees buildconf needs.
+- Find the user's GitHub fork: their login, and the remote that pushes to
+ `<login>/httpd`:
+ ```
+ FORK_OWNER=$(gh api user --jq .login)
+ git remote -v | grep "[:/]$FORK_OWNER/httpd\(\.git\)\? (push)"
+ ```
+ The first column is `<fork-remote>`, used in step 5. If no remote matches, ask the user
+ which remote is their fork; never fall back to `origin`.
+
+## 1. Identify the trunk commits
+
+Record **both** the git hash and the svn revision for every commit.
+
+- Rev → hash (the trailing space stops r123 matching r1234):
+ ```
+ git log origin/trunk --format='%H %s' --grep='trunk@1234567 '
+ ```
+ It must resolve to exactly one commit.
+- Hash → rev:
+ ```
+ git show <hash> --no-patch --format=%b | grep -o 'trunk@[0-9]*'
+ ```
+- Confirm each is on trunk: `git merge-base --is-ancestor <hash> origin/trunk`.
+- Order them oldest first (trunk order), whatever order they were given in.
+
+Check whether any is already in 2.4.x. The repo is mirrored from svn, where a backport is
+a fresh commit, so `git cherry` and `git branch --contains` say nothing; grep the
+`origin/2.4.x` log for the rev (`git log origin/2.4.x --grep='r1234567'`) and compare the
+code itself.
+
+Print a table and confirm it with the user before going further:
+
+| rev | hash | subject |
+| --- | --- | --- |
+| r1234567 | 0123456789 | ... |
+
+## 2. Branch, worktree, configure
+
+Branch `2.4.x-<topic>`; worktree under `.claude/worktrees/` with a name of **7 characters
+or fewer**, e.g. `bpssl`. The `test/modules/arch/linux` suite binds a unix socket under
+the worktree path, which has to fit the 108-byte `sockaddr_un` limit:
+
+```
+git worktree add -b 2.4.x-<topic> .claude/worktrees/<name> origin/2.4.x
+```
+
+In the worktree (a fresh worktree has no `config.nice`):
+
+```
+./buildconf --with-apr=<APR source tree>
+./configure --enable-maintainer-mode --with-ssl --enable-reduced-exports \
+ --enable-modules=reallyall --enable-mpms-shared=all
+```
+
+If the local guidance does not name the APR source tree, ask the user; apr-util is found
+alongside it. Don't pass an `apr-1-config` path to `--with-apr`: that mode copies the
+`.m4` files from `apr-config --installbuilddir`, where some distributions don't ship them.
+**Never `make install`**.
+
+## 3. Cherry-pick each revision into its own commit
+
+In trunk order:
+
+```
+git cherry-pick -x <hash>
+```
+
+Then rewrite the message with `git commit --amend -F <file>` (file in the scratchpad):
+the trunk message is kept **as-is**, except the `git-svn-id:` line is removed and the end
+of the message becomes:
+
+```
+Merge r1234567 from trunk.
+(cherry picked from commit <full hash>)
+```
+
+Trunk trailers (`Submitted by:`, `PR:`, `Github:` etc.) stay where they were, above those
+two lines. No `Co-Authored-By` trailer for a clean pick: the change is the trunk
+author's. Never a `Claude-Session` trailer.
+
+**Always exclude `docs/log-message-tags/`**: it is maintained only on trunk. Drop any
+change to it from the pick (`git rm -f` a file the pick adds, `git checkout HEAD --` one
+it modifies) before committing; the `APLOGNO()` numbers in the code are allocated on trunk
+and are backported unchanged. No omission note is needed for this.
+
+Conflicts:
+
+- Resolve against the 2.4.x code, which differs in APIs, MMN and surrounding context. Read
+ the 2.4.x function before editing; don't force the trunk hunk in.
+- A `CHANGES` hunk should not be carried; bring the trunk `changes-entries/*.txt` file if
+ the rev has one.
+- If a rev depends on an earlier trunk rev not in the list, **stop** and report it,
+ suggesting that rev be added, rather than writing the missing code.
+- Note a resolved conflict in that commit, as a line above the `Merge` line:
+ `[conflicts resolved in path/to/file.c]`. Add `Co-Authored-By` only if the resolution
+ was substantive. Flag it in the report and the PR body.
+
+Omitted files: if the user asks for code only, or a rev's test files don't exist on 2.4.x,
+drop those hunks (`git rm -f` / `git checkout HEAD --`). Then remove the `* docs/...` and
+`* test/...` bullet paragraphs for them from the message, and add no omission note: the
+message describes only what the commit contains.
+
+## 4. Build
+
+From the top level of the worktree:
+
+```
+make -j$(nproc)
+```
+
+Never `make -C modules/<dir>`: it leaves the `.so` stale. Treat a new compiler warning in
+the touched files as a failure. Report any failure verbatim, fix it (a backport-specific
+fix goes in that rev's commit, noted as a conflict resolution) and rebuild.
+
+Offer, but don't run by default, `make check-pytest PYTEST_DIRS=test/modules/<pkg>` for
+the affected area.
+
+## 5. Push, preview the PR, then create it
+
+Push to the user's fork (`<fork-remote>` from step 0) with an **explicit refspec** (a
+bare push resolves to the tracked branch). Never push to `origin`:
+
+```
+git push <fork-remote> 2.4.x-<topic>:2.4.x-<topic>
+```
+
+Draft the PR:
+
+- Subject: `[2.4.x] Merge changes to <topic>`
+- Body: a `Merge ... from trunk:` line, then each trunk message inline-quoted (every line
+ prefixed with `> `, `git-svn-id:` line removed), a blank line between them:
+
+ ```
+ Merge r1234567, r2345678 from trunk:
+
+ > * modules/ssl/ssl_engine_io.c (foo): Do the thing.
+ >
+ > Submitted by: ...
+
+ > * modules/ssl/ssl_engine_kernel.c (bar): Do the other thing.
+
+ 🤖 Generated with [Claude Code](https://claude.com/claude-code)
+ ```
+
+ The quoted messages keep their original line breaks; they are quotes. Any prose
+ written for the PR (e.g. a conflict note) is one line per paragraph, not hard-wrapped:
+ GitHub reflows Markdown to the reader's width.
+
+**Show the subject and body to the user and wait for approval.** Only then:
+
+```
+gh pr create --repo apache/httpd --base 2.4.x --head $FORK_OWNER:2.4.x-<topic> \
+ --title '[2.4.x] Merge changes to <topic>' --body-file <scratchpad file>
+```
+
+Report the PR URL. Leave the worktree in place for follow-ups and give the command to
+remove it: `git worktree remove .claude/worktrees/<name>`.
+
+## 6. STATUS entry
+
+Once the PR exists, give the user an entry to paste into the 2.4.x `STATUS` file, under
+`PATCHES PROPOSED TO BACKPORT FROM TRUNK:`. Output it only; do not edit or commit
+`STATUS`. The shape, 2-space indent for `*)`, 5 for the fields, continuation URLs aligned
+under the first:
+
+```
+ *) mod_foo: One-line CHANGES-style summary of what the backport does.
+ trunk patch: https://svn.apache.org/r1234567
+ https://svn.apache.org/r2345678
+ 2.4.x patch: https://github.com/apache/httpd/pull/NNN.patch
+ +1:
+```
+
+- One `https://svn.apache.org/rNNN` line per revision on the branch, in trunk order,
+ including any added later (dependencies, follow-up fixes).
+- The summary uses the same prefix a CHANGES entry would (`mod_foo:`, `core:`), written
+ for a voter, not a list of the commit subjects.
+- **The `+1:` line MUST be empty**: ` +1:` with nothing after it, not even a trailing
+ space. Votes are added by the committers themselves.
+
+Then offer to copy the entry to the clipboard. On a yes, write it to a file in the
+scratchpad and use whichever tool the session has a display for:
+
+```
+if [ -n "$WAYLAND_DISPLAY" ] && command -v wl-copy >/dev/null; then wl-copy < <file>
+elif [ -n "$DISPLAY" ] && command -v xclip >/dev/null; then xclip -selection clipboard < <file>
+elif command -v pbcopy >/dev/null; then pbcopy < <file>
+fi
+```
+
+If none applies (e.g. an SSH session), say so and leave the entry in the reply to copy by
+hand. Check the file has no trailing whitespace (`grep -n ' $' <file>` finds nothing)
+before copying.
+
+## Notes
+
+- Keep a running record at `.claude/sessions/backport-<topic>-session.md` in the main
+ checkout (not the worktree, which gets removed), appended as work happens. Leave it
+ untracked.
+- A follow-up to a pushed PR is a new commit, never a force-push without explicit
+ confirmation.
Added: httpd/dev-tools/trunk/skills/pr-test/SKILL.md
==============================================================================
--- /dev/null 00:00:00 1970 (empty, because file is newly added)
+++ httpd/dev-tools/trunk/skills/pr-test/SKILL.md Mon Sep 28 14:10:34 2026 (r1938616)
@@ -0,0 +1,236 @@
+---
+name: pr-test
+description: Evaluate a GitHub PR against apache/httpd and prove it with a pyhttpd test - judge the change, then build a red-to-green branch that fails before the PR commit and passes after it. Use for "evaluate PR 529", "review PR NNN", "test a PR", "prove PR NNN fixes the bug".
+---
+
+# Evaluate a PR and prove it with a test
+
+Given a PR number, judge the change, then build a `prNNN-testing` branch whose history
+is: a new pyhttpd test that fails, followed by the submitter's commit(s) applied
+unchanged, after which it passes. The branch is the deliverable — it shows the PR does
+what it claims rather than merely looking plausible.
+
+Usage: `/pr-test 529`, or `/pr-test 529 owner/repo` to override the source repo.
+
+## 0. Preflight
+
+- If the checkout has an `AGENTS.md`, read it first and follow it. Local guidance there
+ (paths, build setup, commit and PR conventions) takes precedence over the defaults in
+ this skill.
+- The PR repo defaults to **`apache/httpd`**, which is what a bare `gh` resolves to here
+ (`origin` is upstream).
+- Find the user's GitHub fork: their login, and the remote that pushes to
+ `<login>/httpd`:
+ ```
+ FORK_OWNER=$(gh api user --jq .login)
+ git remote -v | grep "[:/]$FORK_OWNER/httpd\(\.git\)\? (push)"
+ ```
+ The first column is `<fork-remote>`, used in step 5. If no remote matches, ask the user
+ which remote is their fork; never fall back to `origin`.
+- Working tree must be clean. If it isn't, offer a worktree under `.claude/worktrees/prNNN`
+ — the name must stay **≤ 7 characters**: the `test/modules/arch/linux` suite binds a
+ unix socket under the worktree path, which has to fit the 108-byte `sockaddr_un` limit.
+- The tree must be configured and built: `Makefile` exists and `build/config_vars.mk` sets
+ `PYTEST3`.
+- `git fetch origin`, then base everything on `origin/trunk`. Local `trunk` is stale; never
+ branch off it, and never off another topic branch. If the PR targets `2.4.x`, base on
+ `origin/2.4.x`.
+
+## 1. Evaluate the change
+
+```
+gh pr view NNN --repo apache/httpd --json title,body,author,state,url,commits,files
+gh pr diff NNN --repo apache/httpd
+```
+
+Read the surrounding in-tree code for every hunk — do not judge from the diff alone.
+Assess:
+
+- Is the diagnosis right, and is the fix the right one for it?
+- Is it minimal, or does it carry unrelated churn?
+- MMN / API / ABI impact; whether it is backportable to 2.4.x.
+- httpd conventions: existing `ap_*` / `apr_*` helpers used rather than new local
+ equivalents; no `APLOGNO()` on `APLOG_DEBUG`/`APLOG_TRACE*`.
+
+Report the evaluation to the user.
+
+**Gate:** if the change does not look reasonable, **stop here.** Report the concerns.
+Create no branch and write no test.
+
+## 2. Branch and write the test
+
+```
+git switch -c prNNN-testing origin/trunk
+```
+
+Pick the pyhttpd package from the files the PR touches:
+
+| PR touches | test package |
+| --- | --- |
+| `modules/aaa/` | `test/modules/aaa` |
+| `modules/http2/` | `test/modules/http2` |
+| `modules/metadata/` | `test/modules/metadata` |
+| `modules/proxy/` | `test/modules/proxy` |
+| `modules/ssl/` | `test/modules/ssl` |
+| `modules/md/` | `test/modules/md` |
+| `modules/generators/mod_status.c` | `test/modules/status` |
+| `server/*.c`, generic filters | `test/modules/core` |
+| core API (`ap_*` in `modules/http/http_request.c`, `http_protocol.c`) | `test/modules/core` |
+| HTTP/1 protocol framing, trailers, strictness | `test/modules/http1` |
+
+A PR touching several areas gets a test in **each** affected package. Route by what the
+change *is*, not which directory it sits in: a core function that happens to live under
+`modules/http/` belongs in `test/modules/core`.
+
+If no shipped module exercises the code path, add the hook to a test module rather than
+inventing a config for it. `test/pyhttpd/mod_aptest/mod_aptest.c` is rebuilt by
+`HttpdTestSetup._add_aptest()` and loaded by every suite, so a hook there must be inert
+unless the test asks for it — gate it on a request header. `test/modules/http1/mod_h1test/`
+is the per-suite equivalent. After touching `mod_aptest.c`, run a second suite to confirm
+the change is inert.
+
+If nothing fits, create a package with `__init__.py`, `conftest.py`, `env.py`, and
+`htdocs/` if it needs files; copy `test/modules/core/` as the simplest template. Nothing
+registers a package anywhere — no list in `Makefile.in` or `configure.in`, pytest just
+collects `test/modules/**/test_*.py`. **`__init__.py` is mandatory**: every `conftest.py`
+does `from .env import ...`. `test/README.pytest` documents the layout.
+
+Follow the house pattern — `test/modules/core/test_006_merge_lang.py` is a compact
+example:
+
+```python
+class TestSomething:
+
+ @pytest.fixture(autouse=True, scope='class')
+ def _class_scope(self, env):
+ conf = HttpdConf(env, extras={
+ f"test1.{env.http_tld}": """
+ SomeDirective on
+ """,
+ })
+ conf.add_vhost_test1()
+ conf.install()
+ assert env.apache_restart() == 0
+
+ def test_core_007_01(self, env):
+ r = env.curl_get(env.mkurl("http", "test1", "/doc.html"))
+ assert r.response["status"] == 200
+```
+
+- Name methods `test_<suite>_<NNN>_<NN>`, taking the next free number in the package.
+- Assert on `r.response["status"]`, `r.response["header"][...]` (keys lowercased),
+ `r.response["body"]`.
+- Declare the module under test in the package's `*TestSetup.add_modules()`, or
+ `add_optional_modules()` when it is build-conditional — see `test/modules/metadata/env.py`
+ for the optional-module plus `has_shared_module()` idiom. A module in `add_modules()`
+ with no `.so` aborts setup with "Unable to find modules", so it must be built as a DSO.
+- `test/conftest.py` calls `env.check_error_log()` after every test and fails on any
+ unignored error or warning. For log output the test expects, assert it with
+ `env.httpd_error_log.scan_recent(re.compile(...))` and then clear it with
+ `env.httpd_error_log.ignore_recent(lognos=["AHNNNNN"])`, as
+ `test/modules/metadata/test_001_mime_magic.py` does.
+- `test/modules/status` is gated behind `TEST_STATUS` in its `conftest.py`
+ (`collect_ignore_glob`), so a status test needs that set in the environment.
+
+The test must fail **for the reason the PR describes**. A failure from a config mistake
+proves nothing — check the error log and the response to confirm the mechanism matches.
+
+## 3. Show the test fails
+
+Build from the **top level**:
+
+```
+make -j$(nproc)
+```
+
+`make -C modules/<dir>` does not relink the shared module, and `check/modules/*.so` are
+symlinks to those `.libs` files, so a partial build silently tests old code. Check the
+`.so` timestamp before trusting a result.
+
+```
+make check-pytest PYTEST_DIRS=test/modules/<pkg>
+```
+
+**The path must start with `test/`.** pytest's rootdir is the httpd top directory, so
+`PYTEST_DIRS=modules/aaa` collects 0 items and exits 5 rather than erroring usefully. No
+`make install` is needed — `check-pytest` builds a `check/` prefix and symlinks the built
+modules into `check/modules/`.
+
+If a run is interrupted, the `check/bin/httpd` it started survives and keeps ports
+5001-5004 and 5100 bound, so every later run errors at setup rather than failing cleanly —
+the tell is dozens of ERRORs and a runtime of minutes where a clean run takes seconds.
+`apachectl -k stop` will not help once the pid file is gone, so kill it directly and
+confirm the ports are free before re-running:
+
+```
+pgrep -af "$PWD/check/bin/httpd"
+ss -ltn | grep -E ':500[1-4]|:5100'
+```
+
+Run the whole package, not just the new test, to establish a clean baseline. To narrow
+down afterwards, call the same pytest the target uses (the *system* `pytest-3`, not the
+`uv` venv):
+
+```
+PYHTTPD_CONFIG=$PWD/check/config.ini pytest-3 test/modules/<pkg> -k <name>
+```
+
+Capture the failure output verbatim for the report, then commit the test alone. A
+brand-new test file's message is just:
+
+```
+* test/modules/<pkg>/test_NNN_foo.py: New test suite.
+
+Co-Authored-By: <model> <no...@anthropic.com>
+GitHub: PR #NNN
+```
+
+A case added to an existing file gets the normal one-clause ChangeLog form. Never a
+`Claude-Session` trailer.
+
+## 4. Apply the PR commit AS-IS and show it passes
+
+`origin` has no PR refspec, so fetch the head explicitly:
+
+```
+git fetch origin refs/pull/NNN/head:refs/remotes/origin/pr/NNN
+git cherry-pick -x <sha>...
+```
+
+Cherry-pick every commit in PR order. **AS-IS**: no rewording, no style fixes, no
+squashing, no amending. Original authorship is preserved and the branch shows exactly what
+the submitter proposed. If a commit does not apply cleanly to current trunk, stop and
+report — editing the patch defeats the point.
+
+Rebuild top-level `make -j$(nproc)` and re-run the same `check-pytest` command. The new
+test must now pass **and** nothing else in the package may regress. Capture the output
+verbatim.
+
+If the test still fails, that is the headline result. Report it. Do not massage the test
+until it goes green.
+
+## 5. Push to the fork
+
+Push to the user's fork (`<fork-remote>` from step 0):
+
+```
+git push <fork-remote> prNNN-testing:prNNN-testing
+```
+
+**Use the explicit refspec.** The branch tracks `origin/trunk`, so with `push.default`
+set to `upstream` (or `tracking`) a bare `git push <fork-remote> prNNN-testing` resolves
+to `trunk` on the fork.
+
+**Never push to `origin`** (apache/httpd). Report the branch URL.
+
+Do not open a PR: its subject and body must be shown to the user and approved first.
+Offer to draft one.
+
+## Notes
+
+- Keep a running record at `.claude/sessions/prNNN-testing-session.md` in the main
+ checkout (not a worktree, which gets removed), appended as work
+ happens rather than written up at the end. Leave it untracked.
+- Stay inside this checkout: no `$HOME`, no sibling trees, no scratch directories.
+- Add no `changes-entries/` file. That belongs to the PR author; this branch is their
+ commit plus a test.