Skip to content

Commit dabd759

Browse files
committed
Keep format() but stop it refreshing the snapshot
format(overwrite_file = TRUE) rewrites the Stan file and then reassigns stan_code_ and clears variables_, so both accessors end up describing a source the executable was never built from. That is #1228's failure in the opposite direction. The refresh was added deliberately in #1235, under the older contract where code() meant the file as it is now, and the accessors have since been redefined without it catching up. Deleting those lines is the whole fix, so the method stays and keeps overwriting. A reviewer proposed removing it; rewriting the file is the useful part and is not what breaks anything. Afterwards the file changes, the snapshot keeps describing the built source, and the next operation that runs the binary errors and points at cmdstan_model(). Reformatting therefore forces a recompile, which is correct: the bytes changed, and whether the build is unaffected cannot be known without doing it. Also adds a NEWS reconciliation pass before the release candidate. The unreleased section already carries fifteen-plus entries about $compile(), compile = FALSE and dry_run that stage 4 deletes, and one describing the format() refresh this commit reverses. Such entries are removed rather than annotated, since a user upgrading from 0.9 never saw the intermediate behaviour.
1 parent e39fd0b commit dabd759

1 file changed

Lines changed: 32 additions & 0 deletions

File tree

dev-notes/compilation-state.md

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -712,6 +712,23 @@ parses from disk on first call (`R/model.R:1032`), so an edit made before that f
712712
call would return information about the *new* source while claiming to describe the
713713
built onethe contract violated by the mechanism meant to implement it.
714714

715+
**`$format(overwrite_file = TRUE)` must stop refreshing the cache**
716+
(`R/model.R:1308-1312`). It rewrites the Stan file and then reassigns `stan_code_`
717+
and clears `variables_`, which makes both accessors describe a source the executable
718+
was *never* built from#1228's failure in the opposite direction. #1235 added that
719+
refresh deliberately, under the older contract where `$code()` meant "the file as it
720+
is now"; the accessors have since been redefined and this has not caught up.
721+
722+
Removing those lines is the whole fix. **`$format()` is kept, overwriting included**
723+
a reviewer proposed removing the method, but rewriting the file is the useful part
724+
and is not what breaks anything. With the refresh gone: the file changes, the
725+
snapshot keeps describing the built source, the Stan file's content hash no longer
726+
matches, and the next operation that runs the binary errors and points at
727+
`cmdstan_model()`. So **reformatting forces a recompile**, which is correct rather
728+
than unfortunate — the bytes changed, and whether the build is unaffected cannot be
729+
known without doing it. No warning is needed, because §5 already says this about
730+
external edits and formatting is only an edit cmdstanr performs on the user's behalf.
731+
715732
Capture costs nothing extra: the assessment already invokes `stanc --info` for
716733
include resolution (§6), and the same output carries the variables. The assessment
717734
returns parsed source information; the constructor commits it as the object's
@@ -1181,6 +1198,21 @@ binary reports, this is the only way to ask what an executable actually is, and
11811198
only answer available at all for an unprovenanced one (§7). Landing it during the
11821199
candidate period is fine; landing it after the release is not.
11831200
1201+
### Reconciling NEWS before the candidate
1202+
1203+
`NEWS.md` accumulates entries describing behaviour that later stages remove, and by
1204+
1.0 it would read as a changelog for methods that no longer exist. The unreleased
1205+
section already carries fifteen-plus entries about `$compile()`, `compile = FALSE`
1206+
and `dry_run`, all of which Stage 4 deletes, plus at least one — the
1207+
`$format(overwrite_file = TRUE)` cache refresh (`NEWS.md:94-96`) — describing
1208+
behaviour this document now reverses.
1209+
1210+
**Entries are removed, not annotated.** An entry that no longer applies at 1.0 is
1211+
noise for the reader it was written for, whichever release introduced it: a user
1212+
upgrading from 0.9 to 1.0 never saw the intermediate behaviour and does not need to
1213+
know it existed. This is a pass before the candidate, once the stages have settled,
1214+
not something to do incrementally while the picture is still moving.
1215+
11841216
### The release candidate
11851217
11861218
A 1.0 candidate ships after Stage 4, so downstream packages have something to

0 commit comments

Comments
 (0)