diff --git a/.github/workflows/upstream-branch-guard.yml b/.github/workflows/upstream-branch-guard.yml new file mode 100644 index 00000000..ce9b39a4 --- /dev/null +++ b/.github/workflows/upstream-branch-guard.yml @@ -0,0 +1,102 @@ +# An `upstream/**` branch exists to become a pull request against +# datafusion-contrib/liquid-cache. Our `main` is upstream plus this fork's +# patches, so a branch cut from `main` carries every one of those patches into +# the upstream diff. This fails that on push, before a PR exists, and explains +# why. +name: upstream branch guard + +on: + push: + branches: ['upstream/**'] + +permissions: + contents: read + +jobs: + guard: + name: branch is based on upstream, not on our main + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Check where this branch diverged + run: | + set -euo pipefail + + git fetch --quiet origin main:refs/remotes/origin/main + git fetch --quiet https://github.com/datafusion-contrib/liquid-cache main + upstream=$(git rev-parse FETCH_HEAD) + + # Where this branch and our main last shared history. A branch cut + # from upstream shares only upstream history with our main, so that + # point is a commit upstream has. A branch cut from our main shares a + # fork commit, and upstream never contains one. + # + # Testing the merge base rather than our main's tip matters: `main` + # moves with every merge, so a branch cut from `main` last week has + # today's tip nowhere in its history and would look clean. + base=$(git merge-base HEAD refs/remotes/origin/main) + + if git merge-base --is-ancestor "$base" "$upstream"; then + own=$(git rev-list --count "$upstream"..HEAD) + echo "OK: this branch diverged from upstream history, not from our fork's." + echo "It adds $own commit(s) on top of upstream, which is what an upstream branch should look like." + exit 0 + fi + + carried=$(git rev-list --count "$upstream".."$base") + cat < FETCH_HEAD + git cherry-pick + + HOW TO CHECK BEFORE PUSHING + This must list only the commits you wrote. Anything else in it is + a fork patch that would land in the upstream diff: + + git fetch https://github.com/datafusion-contrib/liquid-cache main + git log --oneline FETCH_HEAD..HEAD + + ONE MORE THING + Reproduce the bug on upstream's tree before opening the PR: apply + your test alone, watch it fail there, then apply the fix. A fix + whose code still exists upstream is not evidence the bug does. + + MSG + echo "::error::Branch is based on our main and carries $carried fork commit(s); an upstream PR from it would include them. See the log above." + exit 1 diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 00000000..be5bc539 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,80 @@ +# Working in this fork + +`hotdata-dev/liquid-cache` is a fork of `datafusion-contrib/liquid-cache`. +`main` is upstream's `0033b15` with our patches on top — upstream's history is +fully contained, so `git merge-base main ` resolves. + +`AGENTS.md` and `README.md` are upstream's and describe the project itself. +This file is ours and describes only what differs here. Nothing in this repo +should edit an upstream-owned file to record a fork convention: that conflicts +on every sync. Add a file upstream does not have instead. + +## Branches + +- Work off `main`. Fetch first — a local `main` goes stale with no signal, and + branching off a stale one silently drops everything merged since. The remote + is `origin` in a fresh clone; if you cloned upstream and added this fork as a + second remote, use that name instead. +- **Upstream PRs branch from upstream, not from `main`**, and are named + `upstream/`. A branch cut from `main` carries our whole patch stack + into the PR diff. + + ``` + git fetch https://github.com/datafusion-contrib/liquid-cache main + git checkout -b upstream/ FETCH_HEAD + ``` + + Before pushing, this must list only the commits you wrote — anything else is + a fork patch that would land in the upstream diff. Re-fetch upstream on the + line above it: `FETCH_HEAD` holds whatever the last fetch wrote, so after a + `git fetch origin` it is *our* `main` and the check hides every fork patch. + + ``` + git fetch https://github.com/datafusion-contrib/liquid-cache main + git log --oneline FETCH_HEAD..HEAD + ``` + + `.github/workflows/upstream-branch-guard.yml` checks the same property on + push, before a PR exists. It tests where the branch diverged rather than + whether it contains `main`'s current tip, because `main` moves with every + merge and a branch cut from it last week contains today's tip nowhere. + +- Before raising an upstream PR, reproduce the bug **on upstream's tree** — + apply the test alone, watch it fail, then apply the fix. A fix whose code + still exists upstream is not evidence the bug does; that mistake has cost us + a withdrawn PR. Several of our patches repair machinery upstream does not + have (`DiskResidue`, `reclaim_orphaned_disk`, `settle`) and are not + upstreamable at all. + +## Building and testing + +- **`cargo +1.96.0`.** The dependency tree needs 1.95+ (`vortex-*`, `sysinfo`) + and DataFusion 55 needs 1.94. A bare `cargo` on an older default fails + resolution with a wall of `requires rustc 1.9x` lines. +- **Shuttle tests need a filter**: `cargo +1.96.0 test -p liquid-cache + --features shuttle --lib shuttle_`. The feature swaps `crate::sync` to + shuttle primitives for the whole test build, so running it unfiltered fails + ~49 unrelated tests with "Are you accessing a Shuttle primitive outside of a + Shuttle test?". That is by design, not a regression. +- **`dev-tools` needs `dev/dev-tools/assets/tailwind.css`**, which CI generates + and the repo does not carry. To run its tests locally, create a placeholder + and delete it before committing. Do not habitually pass + `--exclude dev-tools`: it hides real failures, including trace-snapshot + breakage from new cache events. +- After a structural edit, compare the **test inventory**, not just the pass + count. A `#[cfg(feature = "shuttle")]` test can be deleted without moving the + default-build total at all, and CI stays green because a missing test is not + a failing one. + +## Known open issue + +The disk reclaim path has an unclosed race. A store key is +`(entry id, identity)` and identities are reused — the file-id pool hands a +re-opened path its previous record. `reclaim_orphaned_disk` consults the index +before deleting, but a put that has landed while its index record is not yet +installed is invisible to that check, and t4 applies puts and tombstones by +LSN, so the later `remove` wins and deletes live bytes. + +Closing it needs a per-write generation in the store key, which also makes +`DiskResidue::superseded` unreachable and removes the in-place-overwrite case. +Not yet done. Do not re-report it as new.