Repository navigation
Support overriding diff options in project config (& for individual units) - #263
Merged
Merged
Conversation
freeqaz
added a commit
to freeqaz/objdiff
that referenced
this pull request
Aug 9, 2026
…ually takes effect The premise this started from -- "`report generate` hardcodes four diff options and never consults `project.options`" -- turned out to be false. Upstream encounter#263 (56dac46) already taught `generate` to layer project options, then per-unit options, then `-c key=value` on top of the report base config, via `build_unit_diff_config`. Verified: `-c functionRelocDiffs=name_only` moves the number today (60.00523 -> 60.00392 on a two-obj probe project), and so does an `options` block -- on a cold cache. The actual bug is one layer down: `ReportCache::hash_unit` keyed on obj bytes + `-c` args and NOTHING else. Add `options` to objdiff.json, re-run to the same output path, and every unit comes back out of the `.cache` file diffed under the OLD config. Reproduced against the pre-fix binary: same command, same objs, options set, 1 cache hit, 60.00523 -- the pre-option value. Silent. That is almost certainly why project options looked like they were "never consulted". Fixed by hashing the project and unit option maps too, under separators (0xFD / 0xFC) distinct from the `-c` separator (0xFE) so an `options` entry cannot alias the equivalent CLI arg -- they layer in different orders and can differ in effect. `ProjectOptions` is a BTreeMap, so the key is order-stable. Deliberately appended ONLY when the map is non-empty. A project that sets no options -- which today is all of dc3-decomp, rb3-xenon, ChimpsAtSea_Reach, decomp-clones/halo and cea-decomp -- produces byte-identical key material to the pre-fix function, so existing `.cache` files stay valid: no forced re-diff of 3675 units, and no chance of a number moving for a fix nobody opted into. Verified both directions on the probe project: with options set the new binary misses and yields 60.00392; with options removed it HITS a cache entry written by the old binary and yields 60.00523. Also: - `--global-byte-eq` re-read every obj under the bare base config, so with any project option set it would derive byte signatures from differently-decoded instructions than the per-unit pass used. Now uses `build_unit_diff_config`. - Comment the base config as a FALLBACK, not a policy, naming the five projects whose tracked percentages depend on those four values, so nobody "fixes" them to the schema defaults (name_address / false / false / true) later. - `-c` help now says it exists and that objdiff.json `options` is the persistent equivalent. Left the other three hardcoded values (combineDataSections, combineTextSections, ppcCalculatePoolRelocations) exactly as they are -- they were already overridable by the same layering, so there was nothing to make configurable; a test now pins that all four can be overridden from the project file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
freeqaz
added a commit
to freeqaz/objdiff
that referenced
this pull request
Aug 9, 2026
…t cache key `report generate` has layered the project's and unit's `options` blocks into the effective DiffObjConfig since upstream PR encounter#263 (build_unit_diff_config), but ReportCache::hash_unit never included them in the key. Setting e.g. functionRelocDiffs in objdiff.json was therefore a silent no-op against a warm cache: you opted in, the cached rows were served unchanged, the number did not move, and the natural conclusion was that the option did not work. Worse, a partially-warm cache could serve a mix of rows diffed under two different configs in one report. hash_unit now also hashes the project-level option map (separator 0xFD) and the unit-level one (0xFC), kept distinct from the `-c key=value` separator (0xFE) because those layer in a different order and can differ in effect. ProjectOptions is a BTreeMap, so the key is canonical regardless of JSON ordering. The option maps are appended ONLY when non-empty. That is the compatibility hinge, not an optimisation: every project that sets no options -- which today is all of them: dc3-decomp, rb3-xenon, ChimpsAtSea_Reach, decomp-clones/halo, cea-decomp -- produces byte-identical key material to the pre-fix function, so existing .cache files stay valid and nobody eats a full re-diff, or a moved score, for a fix they did not opt into. Verified: no existing project's numbers change. Deliberately NOT changed: the four base values in generate() that differ from config-schema.json's defaults (function_reloc_diffs None vs name_address, combine_data_sections/combine_text_sections true vs false, ppc_calculate_pool_relocations false vs true). Those are the de-facto scoring semantics that every project's tracked history is measured against; "fixing" them to the schema defaults would silently invalidate every recorded score. They are now documented as an overridable fallback, with that warning in place. 8 new tests: the base/project/unit/CLI layering order, invalid-option rejection, and all three cache-key behaviours (unchanged when empty, changed when set, bool values distinguished). 107 pass in objdiff-cli, 0 fail.
This was referenced Oct 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In
objdiff.json, you can now specify global diff options:See the configuration schema.
Additionally, you can specify these options on a per-unit basis, in case specific architecture options may differ per-unit.