Skip to content
This repository was archived by the owner on Dec 21, 2025. It is now read-only.

Avoid creating intermediate .mk files#28

Closed
Rangi42 wants to merge 1 commit intoISSOtm:masterfrom
Rangi42:deps
Closed

Avoid creating intermediate .mk files#28
Rangi42 wants to merge 1 commit intoISSOtm:masterfrom
Rangi42:deps

Conversation

@Rangi42
Copy link
Contributor

@Rangi42 Rangi42 commented May 6, 2025

My goal here is to make issue #1 a moot point, since no file timestamps will be involved.

Instead of storing the rgbasm -M output in a .mk file for each .o and then includeing all those .mk files in the Makefile, this just directly dumps the rgbasm -M output into the Makefile.

This is similar to how pokecrystal's Makefile works, using scan_includes instead of rgbasm -M.

It has to work around the fact that $(shell) unavoidably turns newlines into spaces, by tring them into pipe characters and then substituting to undo the transformation. (The pipe | is highly unlikely to be in any Linux filenames, and is outright forbidden on Windows.)

I did have an issue with building this from scratch on macOS, where make would fail (Error opening INCBIN file 'assets/crash_font.1bpp.pb8': No such file or directory) but running it again would succeed. This was because rgbasm -M was not outputting the assets/crash_font.1bpp.pb8 target unless assets/crash_font.1bpp.pb8.size existed. I think that's a bug with RGBDS; see gbdev/rgbds#903 (comment). Anyway, having .pb8.size depend on .pb8 directly fixes it too.

Copy link
Owner

@ISSOtm ISSOtm left a comment

Choose a reason for hiding this comment

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

This will not work for nested dependencies, nor RGBDS if there are two generated files necessary in a row. RGBASM needs to be run in a loop until the object file exists.

@Rangi42
Copy link
Contributor Author

Rangi42 commented May 6, 2025

How does the previous design avoid the need to run rgbasm in a loop?

Would resolving gbdev/rgbds#903 (comment) be sufficient?

As for nested dependencies, I don't see how the current design handles them either. Every .asm file gets a .o+.mk file, and only .inc files are supposed to be INCLUDEd. If foo.inc then has INCLUDE "bar.inc", when/how would it ever run rgbasm -MG bar.inc?

@ISSOtm
Copy link
Owner

ISSOtm commented May 7, 2025

How does the previous design avoid the need to run rgbasm in a loop?

It doesn't:

[...] this will cause Make to “reload” its dependency information from the modified .mk file, (try to) make the newly discovered dependencies, and then re-run RGBASM, until the .o file is successfully created!

That behaviour is built into Make.

Would resolving gbdev/rgbds#903 (comment) be sufficient?

No. You need to re-build the build graph after discovering new dependencies.

As for nested dependencies, I don't see how the current design handles them either. Every .asm file gets a .o+.mk file, and only .inc files are supposed to be INCLUDEd. If foo.inc then has INCLUDE "bar.inc", when/how would it ever run rgbasm -MG bar.inc?

It wouldn't; instead, it would re-run rgbasm -MG foo.asm, and transitively discover the dependencies of bar.inc via foo.asm's INCLUDE "bar.inc".

@Rangi42
Copy link
Contributor Author

Rangi42 commented May 7, 2025

Thanks for running the CI. It does at least now build on Windows, and on macOS without having to brew install make, so shall we leave this PR open as a draft? If I can ensure that it handles (1) nested deps and (2) multiple nonexistent deps in a row, it would be a a functional replacement here.

@Rangi42 Rangi42 marked this pull request as draft May 7, 2025 03:06
@ISSOtm
Copy link
Owner

ISSOtm commented May 7, 2025

CI passes with this change because the default project only has a single generated dependency at all; if a more complex project (I could think of Shock Lobster, Esprit, and Rhythm Land) can build successfully with this change, then it would be appropriate to merge.

@Rangi42
Copy link
Contributor Author

Rangi42 commented May 8, 2025

I don't see a way to avoid the need for running rgbasm -M in a loop. Not every project can use -MC, and even the ones that can might have nested recursive dependencies (e.g. INCLUDE "this-needs-generating.inc" where this-needs-generating.inc will have INCBIN "this-also-needs-generating.2bpp"). And if it has to be run in a loop, using make's change detection for the .mk file is a clean way to do it (apart from the macOS and Windows issues).

@Rangi42 Rangi42 closed this May 8, 2025
@Rangi42 Rangi42 deleted the deps branch May 8, 2025 18:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments