diff --git a/scripts/__tests__/verify-android-payload.test.js b/scripts/__tests__/verify-android-payload.test.js index 0910a29c2..6b0b58ee6 100644 --- a/scripts/__tests__/verify-android-payload.test.js +++ b/scripts/__tests__/verify-android-payload.test.js @@ -98,23 +98,19 @@ const REQUIRED_HEXAGON_SYMBOLS = [ ]; /** - * A `.dynsym` with `matchCount` entries containing "hexagon", of which the two - * required ones are defined unless `withRequired` is false, plus non-matching - * noise so the pattern count is not simply the symbol count. + * A `.dynsym` carrying the two required symbols as defined entries unless + * `withRequired` is false, plus hexagon-named and non-matching noise so a + * reader that keys off name substrings rather than the declared names would + * reach a different verdict. */ -function hexagonDynsym(matchCount, {withRequired = true} = {}) { +function hexagonDynsym({withRequired = true} = {}) { const symbols = []; if (withRequired) { for (const name of REQUIRED_HEXAGON_SYMBOLS) { symbols.push({name, defined: true}); } } - while (symbols.length < matchCount) { - symbols.push({ - name: `lm_ggml_hexagon_session_${symbols.length}`, - defined: true, - }); - } + symbols.push({name: 'lm_ggml_hexagon_session_init', defined: true}); symbols.push({name: 'lm_ggml_backend_reg_count', defined: true}); symbols.push({name: 'malloc', defined: false}); return buildElf(symbols); @@ -152,9 +148,7 @@ function conformingEntries(prefix = '') { entries[`${prefix}${asset}`] = buildDspStub(); } for (const rule of abi.requiredSymbols) { - entries[`${prefix}lib/${abi.abi}/${rule.lib}`] = hexagonDynsym( - rule.expectedMatchCount.count, - ); + entries[`${prefix}lib/${abi.abi}/${rule.lib}`] = hexagonDynsym(); } } return entries; @@ -255,7 +249,7 @@ describe('a conforming artifact', () => { const entries = conformingEntries('base/'); entries[ 'base/lib/arm64-v8a/librnllama_v8_2_dotprod_i8mm_hexagon_opencl.so' - ] = hexagonDynsym(0, {withRequired: false}); + ] = hexagonDynsym({withRequired: false}); const archive = writeArchive('app-prod-release.aab', entries); const {status, output} = runGate(['--aab', archive]); expect(status).toBe(1); @@ -276,9 +270,7 @@ describe('the Hexagon backend', () => { it('fails when the required symbols are absent, naming both', () => { const entries = conformingEntries(); entries['lib/arm64-v8a/librnllama_v8_2_dotprod_i8mm_hexagon_opencl.so'] = - hexagonDynsym(0, { - withRequired: false, - }); + hexagonDynsym({withRequired: false}); const {status, output} = gateApk(entries); expect(status).toBe(1); for (const name of REQUIRED_HEXAGON_SYMBOLS) { @@ -300,20 +292,7 @@ describe('the Hexagon backend', () => { expect(output).toContain('MISSING lm_ggml_backend_hexagon_reg'); }); - it('fails on a changed symbol count, and says to re-declare it', () => { - const entries = conformingEntries(); - entries['lib/arm64-v8a/librnllama_v8_2_dotprod_i8mm_hexagon_opencl.so'] = - hexagonDynsym(18); - const {status, output} = gateApk(entries); - expect(status).toBe(1); - expect(output).toContain('18 .dynsym entries matching "hexagon"'); - expect(output).toContain('re-declare'); - expect(output).toContain('expectedMatchCount as 18'); - }); - - it('names the llama.rn version the count was read from', () => { - const {count, pattern} = manifest.abis.find(abi => abi.abi === 'arm64-v8a') - .requiredSymbols[0].expectedMatchCount; + it('names the llama.rn version the symbols were read from', () => { const installed = JSON.parse( fs.readFileSync( path.join( @@ -330,8 +309,7 @@ describe('the Hexagon backend', () => { const {status, output} = gateApk(conformingEntries()); expect(status).toBe(0); - expect(output).toContain(`${count} .dynsym entries matching "${pattern}"`); - expect(output).toContain(`(declared ${count}), llama.rn ${installed}`); + expect(output).toContain(`.dynsym entries, llama.rn ${installed}`); }); it('reads unknown when llama.rn is not installed, and still passes', () => { @@ -562,44 +540,14 @@ describe('a check that cannot run', () => { // a library but asserts nothing about it therefore restores the incident with // CI green, and counting rules rather than reading them would not notice. it.each([ - [ - 'a rule that asserts nothing', - rule => { - delete rule.mustExport; - delete rule.expectedMatchCount; - }, - ], + ['a rule that asserts nothing', rule => delete rule.mustExport], [ 'a rule whose mustExport has been emptied', rule => { rule.mustExport = []; - delete rule.expectedMatchCount; }, ], ['a rule naming no library', rule => delete rule.lib], - // count: 0 does not merely assert nothing, it asserts the backend is - // ABSENT — so the incident build satisfies it exactly. - [ - 'a rule whose only demand is a count of zero', - rule => { - rule.mustExport = []; - rule.expectedMatchCount = {pattern: 'hexagon', count: 0}; - }, - ], - [ - 'a rule whose expectedMatchCount has no pattern', - rule => { - rule.mustExport = []; - rule.expectedMatchCount = {count: 16}; - }, - ], - [ - 'a rule whose expectedMatchCount is an empty object', - rule => { - rule.mustExport = []; - rule.expectedMatchCount = {}; - }, - ], ])('fails on %s', (_label, weaken) => { const weakened = path.join(workspace, 'weakened.json'); const edited = JSON.parse(JSON.stringify(manifest)); @@ -608,7 +556,7 @@ describe('a check that cannot run', () => { const entries = conformingEntries(); entries['lib/arm64-v8a/librnllama_v8_2_dotprod_i8mm_hexagon_opencl.so'] = - hexagonDynsym(0, {withRequired: false}); + hexagonDynsym({withRequired: false}); const archive = writeArchive('app-prod-release.apk', entries); const {status, output} = runGate([ diff --git a/scripts/android-payload-manifest.json b/scripts/android-payload-manifest.json index 759cd4f25..b2987846c 100644 --- a/scripts/android-payload-manifest.json +++ b/scripts/android-payload-manifest.json @@ -1,7 +1,6 @@ { "$comment": "What a shipped Android artifact must contain. Read by scripts/verify-android-payload.js. See https://github.com/a-ghorbani/pocketpal-ai/issues/858 for why the Hexagon symbol rules exist.", "matching": { - "countRule": "expectedMatchCount counts every .dynsym entry whose name contains the pattern, case-insensitively, including undefined imports. The declared numbers were measured with `llvm-nm -D | grep -ci `, so the reader must match that convention or the two are calibrated differently.", "mustExportRule": "mustExport names must be DEFINED in .dynsym (st_shndx != SHN_UNDEF). An undefined import proves only that something references the symbol, not that it was compiled in.", "extraLibsRule": "librnllama* libraries beyond requiredLibs are reported and permitted, not failed: forcing prebuilts (ORG_GRADLE_PROJECT_rnllamaBuildFromSource=false) legitimately produces all seven arm64 variants.", "assetScopeRule": "requiredAssets paths are NOT ABI-scoped: Android packages assets/ once per artifact, not per ABI (verified against a built APK). They are declared under the ABI whose libraries need them, which is what ties the accelerator floor to the right ABI, but the path is resolved from the artifact root.", @@ -36,11 +35,7 @@ "mustExport": [ "lm_ggml_backend_hexagon_reg", "lm_ggml_backend_is_hexagon" - ], - "expectedMatchCount": { - "pattern": "hexagon", - "count": 16 - } + ] } ], "requiredAssetElfMachine": 164 diff --git a/scripts/verify-android-payload.js b/scripts/verify-android-payload.js index 1f6ed84a4..da5fbf4a3 100644 --- a/scripts/verify-android-payload.js +++ b/scripts/verify-android-payload.js @@ -94,10 +94,7 @@ function parseArgs(argv) { * A symbol rule has to demand that something is *present*. The shape a * weakening edit takes is emptying `mustExport` during a dependency bump * instead of re-declaring it, and the library itself is still there, so no - * other rule notices. `expectedMatchCount` only counts as a demand when it - * asks for a positive number of matches: `count: 0` asserts the backend is - * absent, which is both self-contradictory next to `mustExport` and exactly - * the state this check exists to reject. + * other rule notices — so a rule that names no symbol is refused outright. */ function assertRuleDemandsSomething(rule, manifestPath) { const named = rule.lib || '(unnamed library)'; @@ -110,27 +107,10 @@ function assertRuleDemandsSomething(rule, manifestPath) { if (!rule.lib) { refuse('names no library'); } - const mustExport = rule.mustExport || []; - if (!Array.isArray(mustExport)) { + if (!Array.isArray(rule.mustExport)) { refuse('has a mustExport that is not a list'); } - - const count = rule.expectedMatchCount; - if (count !== undefined) { - if ( - typeof count !== 'object' || - count === null || - typeof count.pattern !== 'string' || - count.pattern.length === 0 || - !Number.isInteger(count.count) || - count.count < 0 - ) { - refuse('has a malformed expectedMatchCount'); - } - } - - const demandsPresence = mustExport.length > 0 || (count && count.count > 0); - if (!demandsPresence) { + if (rule.mustExport.length === 0) { refuse('asserts nothing'); } } @@ -464,12 +444,14 @@ function checkSymbolRule({rule, archive, artifactName, entry, report, fail}) { return; } - report.push(` ${rule.lib}: ${symbols.length} .dynsym entries`); + report.push( + ` ${rule.lib}: ${symbols.length} .dynsym entries, llama.rn ${installedLlamaRnVersion()}`, + ); - const missing = (rule.mustExport || []).filter( + const missing = rule.mustExport.filter( name => !symbols.some(symbol => symbol.name === name && symbol.defined), ); - for (const name of rule.mustExport || []) { + for (const name of rule.mustExport) { report.push( ` ${missing.includes(name) ? 'MISSING' : 'present'} ${name}`, ); @@ -485,29 +467,6 @@ function checkSymbolRule({rule, archive, artifactName, entry, report, fail}) { ].join('\n '), ); } - - const expected = rule.expectedMatchCount; - if (!expected) { - return; - } - const pattern = expected.pattern.toLowerCase(); - const matched = symbols.filter(symbol => - symbol.name.toLowerCase().includes(pattern), - ).length; - report.push( - ` ${matched} .dynsym entries matching "${expected.pattern}" (declared ${expected.count}), llama.rn ${installedLlamaRnVersion()}`, - ); - if (matched !== expected.count && missing.length === 0) { - fail( - [ - `${entry} in ${artifactName} has ${matched} .dynsym entries matching "${expected.pattern}";`, - `scripts/android-payload-manifest.json declares ${expected.count}.`, - 'The required symbols are all present, so the backend is compiled in — this is a drift', - 'tripwire, not a breakage. If the change is expected (a llama.rn upgrade, say), re-declare', - `expectedMatchCount as ${matched} in the same pull request so the diff is reviewed.`, - ].join('\n '), - ); - } } function checkArtifact({archive, kind, manifest, report, failures}) {