RELEASING.md step 1 verifies the installed package, not the release under test

[parisek]

Summary

RELEASING.md step 1 is described as the one check CI cannot run, to be done locally before tagging:

ddev exec "php vendor/bin/acf-schema-gen --wp-root /var/www/html --output /tmp/acf-schemas-out/"
diff -r /tmp/acf-schemas-out/ vendor/parisek/acf-json-schema/schemas/

Run that way it verifies the installed copy of the package, not the working copy being released. Any hand-curated ref change in the release under test is invisible to it.

Mechanism

acf-schema-gen boots WordPress (--wp-root). WordPress loads the theme, the theme's Composer autoloader registers, and Parisek\AcfJsonSchema\* resolves to the theme's installed copy. Confirmed by reflection while preparing v0.7.6, running the generator from a checkout of the release branch against a DDEV site whose theme requires this package:

SchemaEmitter loaded from:
/var/www/html/wp-content/themes/<theme>/vendor/parisek/acf-json-schema/src/Emit/SchemaEmitter.php

copyStaticRefs() (src/Emit/SchemaEmitter.php:162) copies from __DIR__ . '/../templates/refs' — which, with that class loaded, is the installed package's templates. So the output carries the previously released refs.

Consequence

The diff shows the release's own hand-curated changes as differences, in the direction that reads as "my edit did not make it into the generated output". Step 1's own instruction — "If diff shows changes, review them" — then invites reverting the very change being shipped.

Concretely, preparing #38 the diff was:

< "first_day": { "type": ["integer", "string"] }
---
> "first_day": { "type": ["integer", "string"] },
> "readonly": { "enum": [0, 1], … },
> "disabled": { "enum": [0, 1], … }

Every entry on the > side was the change under release. Nothing was actually wrong.

It also means the check silently under-delivers on its stated purpose: with the old code loaded, it compares the previous release against live ACF. That still catches ACF version drift — which is most of the value, and why this has not bitten before — but it cannot catch drift introduced by the release itself.

Suggested direction

Something that pins which copy runs, e.g.:

• run the generator against a WP install that does not require the package, or
• assert the loaded class path up front and fail if it is not the working copy — cheap, and it turns a silent wrong answer into a loud one, or
• note the trap in RELEASING.md so the reviewer knows to expect the release's own edits on the > side.

The third alone would have been enough here; I only noticed because the diff was suspiciously exactly my own change.

No PR opened — the right fix depends on how you want the check to run, and the doc change and the code change are different calls. Happy to do either.

Found while releasing v0.7.6.

[parisek]

Correction and additions after review

The symptom in the report is right; three things around it are not.

1. The mechanism I gave is wrong

I wrote that WordPress loading the theme causes its autoloader to win. The ordering alone would not do that — the working copy's autoloader registers first (bin/acf-schema-gen), and Composer normally appends.

The actual cause is that the theme's Composer bootstrap prepends: wp-content/themes/<theme>/vendor/composer/autoload_real.php:33 calls

$loader->register(true);

so its autoloader is consulted before the working copy's, for any class not yet loaded.

2. It is not only RELEASING.md — Generator has the trap, and so does SnapshotTest

Generator::run() boots WordPress at src/Generator.php:25, then instantiates the extractors (:28-30) and SchemaEmitter (:32). Every one of those classes is first referenced after the prepended autoloader exists, so all of them resolve to the installed copy.

tests/SnapshotTest.php:84 drives Generator the same way. With ACF_SCHEMA_TEST_WP_ROOT pointed at a site whose theme requires this package, the release-time gate from ADR 0001 has the same hole — and unlike the manual step, nobody is reading its diff.

So a doc-only fix, which I suggested might be enough, would leave the automated gate broken. Withdrawing that.

The fix that removes the trap rather than detecting it: force-load the package's own classes — SchemaEmitter, the three extractors, Json — before bootstrapWordPress(). PHP will not redefine an already-loaded class, so the prepended autoloader cannot reach them. RELEASING.md step 1 then collapses to ACF_SCHEMA_TEST_WP_ROOT=… composer test, which also deletes the hand-run duplicate of SnapshotTest.

3. The documented diff target is also the installed copy

Step 1 says:

diff -r /tmp/acf-schemas-out/ vendor/parisek/acf-json-schema/schemas/

From a package checkout that path does not exist — the package does not require itself. From a theme it is the installed copy. So as written, both sides of the diff are the installed package. I saw the release's own edits only because I diffed against the working copy's schemas/ instead. Worth fixing both halves, or the next reader fixes one and keeps the other.

4. What step 1 can actually verify

I implied the step is meaningful for refs — that a hand-curated ref change "is invisible to it". That is backwards. copyStaticRefs() (src/Emit/SchemaEmitter.php:162) copies the package's own templates verbatim, so the refs half of the diff is self-vs-self by design and can never catch ACF drift. Only the extractor-backed roots, the acf.schema.json / field-item.schema.json composition and verifyAcfPro() touch live ACF.

A ref change can therefore only surface in that diff through this bug — which is exactly what happened. Saying so in the doc would make the step's real scope obvious.

Happy to open the PR for the force-load plus the doc corrections if that shape suits.