Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Makefile: Add clean_builtsrc, clean_builtassets #1444

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Dragorn421
Copy link
Collaborator

TLDR

This PR adds clean_builtsrc (to rebuild src/),
and clean_builtassets (to rebuild assets/)
make clean && make : 3min40s
make clean_builtsrc && make : 1min40s
make clean_builtassets && make : 2min10s

Motivation

make clean is often used when building (make) fails for some obscure reason, or the ROM doesn't build OK for equally obscure reasons

Building from clean (make after make clean) takes a long time, 3min40s for me without -j.
The "clean" operation can however be separated into two parts: cleaning built assets (in build/assets) and built source (in build/src)
It is very rarely useful or required to rebuild all of both of these, and typically rebuilding all of either is enough

This PR adds clean_builtsrc and clean_builtassets alongside clean.
Where clean removes the whole build folder of intermediate files, clean_builtsrc focuses on build/src and clean_builtassets on build/assets (they both also remove other things clean removes, see the Makefile)

Building from clean_builtsrc (i.e. re-building src/) is 1min40s for me, half the time to build from clean (still make without -j)
And building from clean_builtassets (i.e. re-building assets/) is 2min10s for me

Makefile Outdated
@@ -251,6 +251,14 @@ endif
clean:
$(RM) -r $(ROM) $(ELF) build

clean_builtsrc:
$(RM) -r $(ROM) $(ELF) build/src build/data
find build -maxdepth 1 -type f -exec rm {} \;
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this find doing exactly?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's removing files directly inside build/, i.e.
dmadata_table_spec.h ldscript.txt spec undefined_syms.txt z64.map (in build/)

I could also put that list of files into the makefile tbh but it felt less random to remove all files

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

(fwiw this is not advanced find usage at all, assuming this may get in and if we keep it I'll add a comment on it though)

Copy link
Contributor

@hensldm hensldm left a comment

Choose a reason for hiding this comment

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

I'm fine with this implementation, but thoughts on instead of new targets, use clean with an env variable (i.e. clean TARGET=builtsrc). I'll admit does seem a little more clunky.

# Delete files directly inside build/ (but not in subfolders)
find build -maxdepth 1 -type f -exec $(RM) {} \;

clean_builtassets:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
clean_builtassets:
builtassetsclean:

Wonder if this would be better to mirror assetclean? (same idea with clean_builtsrc)
Or perhaps builtassets_clean and assets_clean?

@fig02 fig02 added Waiting for author There are requested changes that have not been addressed and removed Needs contributor approval labels Aug 27, 2023
@zeldaret zeldaret deleted a comment from vitaninten Feb 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Waiting for author There are requested changes that have not been addressed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants