pret / pokecrystal

Disassembly of Pokémon Crystal
https://pret.github.io/pokecrystal/
2.06k stars 770 forks source link

`make` defaults to quiet output #1098

Closed Rangi42 closed 7 months ago

Rangi42 commented 7 months ago

This may help beginners who don't notice success/error messages among all the make noise. Experienced users can run make Q=, or modify their Makefile to do so by default.

$ make clean
Deleting build products...
Deleting intermediate build products...

$ make compare -j
Building tools...
Checking RGBDS version...
Finding VC patch values...
Building ROM pokecrystal_debug.gbc...
Building ROM pokecrystal11_debug.gbc...
Building ROM pokecrystal.gbc...
Building ROM pokecrystal11.gbc...
Building ROM pokecrystal11_vc.gbc...
Building ROM pokecrystal_au.gbc...
Building VC patch pokecrystal11.patch...
Comparing output ROMs with originals...
pokecrystal.gbc: OK
pokecrystal11.gbc: OK
pokecrystal_au.gbc: OK
pokecrystal_debug.gbc: OK
pokecrystal11_debug.gbc: OK
pokecrystal11.patch: OK

$ make -j
Building tools...
make: Nothing to be done for 'all'.

The downside is that seeing all those individual commands is a definite indicator that progress is happening; but I think just waiting quietly is also okay, since the process isn't going to infinitely loop/hang, there'd be a visible error message. (Also, even currently, there's a rather long wait period before any messages show up, I think because scan_includes is running.)

I'm not committed to this change, just wanted feedback on a concrete implementation. Like if those explicit echoed messages should be edited or have more/fewer of them.

Rangi42 commented 7 months ago

(Even if we don't want this by default, I'd like to leave all the Qs in so you can explicitly do make Q=@.)

Rangi42 commented 7 months ago

(As for when there are error messages from users' modifications, @ISSOtm 's rsgbds :crab: rewrite will be making those more readable too. :heart: )

Rangi42 commented 7 months ago

Question: Should CI have "un-quiet" output? Again, if/when there is an error, we'd see it regardless, so I don't know if seeing the individual commands is useful or just noise to scroll through in GitHub's lazy-loading log viewer.

dannye commented 7 months ago

Personally, I'm not a fan. I'm fine with the structure being put in place, but I don't want it quiet by default.

Rangi42 commented 7 months ago

Okay. In that case, we don't even need the $(Q)s since make supports -s/--silent for opt-in quiet. Just the @echos so even when silent, you'll see some progress. (I would still like other devs' feedback here.)

ISSOtm commented 7 months ago

Two middle grounds: µcity echoes merely rgbasm file.asm (downside: that looks like a command but it's not), and kernel projects instead display shorthands like ASM file.asm (downside: it's not very clear what those are).

Also, manually outputting commands in that way wouldn't honor -s unless scanning MAKEFLAGS.

mid-kid commented 7 months ago

Not a fan in general, especially for a project that builds from the top-level and has little command line noise like this one. Debugging any sort of build system issue will always involve re-running the entire thing, which might not trigger the same issue again.

That said, I'd consider something like this for out-of-tree builds with a more bespoke makefile, as the paths wouldn't make sense anymore.

Rangi42 commented 7 months ago

Alright, thanks for the feedback!