Skip to content

Commit 681a7dc

Browse files
alexmmillerclaude
andauthored
ADFA-6172: Keep Pebble whole through R8, so templated docs pages render (#2056)
Release builds minify the app module, and R8's shrinker removes members Pebble only reaches by reflection. Two of its three reflective paths were broken: - ExpressionParser instantiates unary operator nodes from a class literal (CoreExtension registers `new UnaryOperatorImpl("not", 500, UnaryNotExpression.class)`, the parser calls Class.newInstance() later). With no traceable `new`, R8 strips the constructor and both evaluate() overrides and marks the class abstract, so a template containing `{% if not x %}` fails to parse with "java.lang.Class<...UnaryNotExpression> cannot be instantiated". - MemberCacheUtils resolves template attributes with getField()/getMethods(), which is how `loop.last` reaches ForNode$LoopVariables. Nothing calls its five getters from bytecode, so R8 removes them and `loop.last` evaluates to null with no error - wrong output rather than a crash. layout.pebble hits both: 3,241 of the templated pages in documentation.db render through it, and it uses `{%- if not loop.last %}`. Keep the whole library rather than the two classes we caught. Attribute resolution makes anything a layout can name reachable invisibly - both expression hierarchies, LoopVariables and its lazy values, the `template` and `_context` globals, and every filter, test and function a future template might use - and BinaryOperatorImpl still exposes the same Class-based constructor to any Extension that ZipRecipeExecutor loads through ServiceLoader. Enumerating that surface is how this recurs. Measured against R8 8.8.34: ~31 KB more than keeping only the two known classes. Verified with R8 8.8.34, the version AGP 8.8.2 uses, run against this rules file: an 80-check sweep of Pebble's operators, filters, tests, functions, tags and attribute access produces output identical to an unshrunk run, where the previous rules gave four parse crashes and silently empty loop variables. Debug builds do not minify, so this only changes release output. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 12ce101 commit 681a7dc

1 file changed

Lines changed: 35 additions & 0 deletions

File tree

‎app/proguard-rules.pro‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,41 @@
188188
*;
189189
}
190190

191+
## Pebble templates (documentation pages, and project templates via ZipRecipeExecutor)
192+
# Pebble reaches a lot of itself reflectively, so R8's shrinker can gut classes a template
193+
# depends on. There are three mechanisms, and two of them bit us:
194+
#
195+
# 1. ExpressionParser instantiates unary operator nodes from a class literal:
196+
# CoreExtension registers `new UnaryOperatorImpl("not", 500, UnaryNotExpression.class)`
197+
# and the parser later calls Class.newInstance(). With no traceable `new`, R8 strips the
198+
# constructor and both evaluate() overrides and marks the class abstract, so a template
199+
# containing `{% if not x %}` fails to parse with "java.lang.Class<...UnaryNotExpression>
200+
# cannot be instantiated" -- observed serving k/kotlin-stdlib/kotlin/apply.html.
201+
# 2. MemberCacheUtils resolves every template attribute by getField()/getMethods(). That is
202+
# how `loop.last` reaches ForNode$LoopVariables, whose five getters nothing calls from
203+
# bytecode. R8 removes them and `loop.last` evaluates to null with no error at all --
204+
# wrong output rather than a crash, which is harder to notice. layout.pebble renders 3,241
205+
# of the templated pages in documentation.db and uses `{%- if not loop.last %}`.
206+
# 3. BinaryOperatorImpl has the same Class-based constructor as (1). CoreExtension happens to
207+
# register binary operators as `OrExpression::new` suppliers, which R8 traces, so nothing
208+
# is broken today -- but it is public API a custom Extension can reach, and
209+
# ZipRecipeExecutor loads template-supplied Extensions through ServiceLoader.
210+
#
211+
# Attribute resolution means anything a layout can name is reachable invisibly: both expression
212+
# hierarchies, LoopVariables and its LazyLength/LazyRevIndex values, the `template` and
213+
# `_context` globals (PebbleTemplateImpl and GlobalContext), and every filter, test, function
214+
# and node type a future template might use. Enumerating that list is how you get a fourth
215+
# instance of this bug, so keep the library whole instead. Measured cost against R8 8.8.34:
216+
# ~31 KB over keeping only the two classes we caught, ~39 KB over keeping nothing.
217+
-keep class io.pebbletemplates.pebble.** { *; }
218+
219+
# The one thing the rule above cannot cover: objects *we* hand to PebbleEngine.evaluate() are
220+
# read by the same reflection, so a POJO or Kotlin data class in a template context needs its
221+
# own keep or its properties render empty. Both call sites today
222+
# (DocumentationContentSource.renderNamed and ZipRecipeExecutor.renderTemplateEntry) pass
223+
# Map<String, ...> of JDK types, which Pebble's MapResolver reads without reflection, so no
224+
# app class needs a rule yet. Add one here alongside any new context type.
225+
191226
## GlitchTip crash reporting (via the Sentry SDK; GlitchTip speaks the Sentry protocol)
192227
-keepattributes SourceFile,LineNumberTable
193228
-keep class io.sentry.** { *; }

0 commit comments

Comments
 (0)