Skip to content

doxygen_symbol_gen: take the map path from the version being built - #776

Open
quarrel07 wants to merge 2 commits into
n64decomp:masterfrom
quarrel07:doxygen-map-path
Open

doxygen_symbol_gen: take the map path from the version being built#776
quarrel07 wants to merge 2 commits into
n64decomp:masterfrom
quarrel07:doxygen-map-path

Conversation

@quarrel07

@quarrel07 quarrel07 commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Every version's ROM rule runs this script, but it hardcodes build/us/mk64.us.map. A fresh clone that builds anything other than us first fails on the last line of the recipe, after the ROM it just produced is already correct:

make assets
make VERSION=eu.v10
...
FileNotFoundError: 'build/us/mk64.us.map'

It now takes the map path as a required argument. Both call sites in the Makefile pass build//mk64..map, and the script exits with a clear message if the argument is missing or the file is not there.

Verified on a fresh clone: us builds and matches, and eu.v10 gets past the step it used to die on.

Every version's ROM rule runs doxygen_symbol_gen.py, but the script
hardcodes build/us/mk64.us.map. A fresh clone that builds anything other
than us first therefore fails on the last line of the recipe, after the
ROM it just produced is already correct.

The script now takes the map as an argument and still defaults to the us
one, so anything calling it bare keeps working.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comment thread tools/doxygen_symbol_gen.py Outdated
# Every version's ROM rule runs this, so take the map of the version being
# built. Hardcoding the us one means a fresh clone that builds any other
# version first fails here, after its ROM is already correct.
map_file_path = sys.argv[1] if len(sys.argv) > 1 else "build/us/mk64.us.map"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if I like that any input is allowed.

Shouldn't we only allow specific input such as us, eu, jp and the script figures out where to go? Or is this the best way?

@quarrel07

quarrel07 commented Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

Fair question. I pushed 22d04be in response, which drops the fallback you were looking at and makes the argument required, rather than adding a version list to the script. Reasoning for going that way:

The version allowlist already exists in the Makefile:

$(eval $(call validate-option,VERSION,us eu.v10 eu.v11))

That errors on anything not in the list, and the path is mechanical from there (BUILD_DIR := build/$(VERSION), TARGET := mk64.$(VERSION)). So by the time the script runs, the version is already known good and Make already knows the exact path.

My worry with moving the list into the script is that it then lives in two places. Every new version has to be added to both, and the script would have to rebuild build/<version>/mk64.<version>.map itself, so a future change to the build layout breaks docs quietly. That is the same shape as the bug this PR is fixing. The names are also finer grained than us/eu/jp, it is us, eu.v10, eu.v11 today, jp.v10/jp.v11 once #777 lands, and in theory iQue variants after that.

So in the new commit:

  • the argument is required, no silent fallback to the us map if something calls it bare
  • it checks the file is there and exits with a real message instead of a traceback

Both call sites already pass the path, so nothing else needed touching. Checked it by hand with no argument, a missing file, and a real eu.v10 map. Happy to switch to a version name if you would still rather have it that way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants