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

src/Makefile: avoid verbosity checks inside targets #154

Merged
merged 1 commit into from
Dec 30, 2023

Conversation

ia
Copy link
Contributor

@ia ia commented Dec 29, 2023

TL; DR: use a trick allowed by GNU make to avoid verbosity checks inside targets. Also, unify output according to logic of rules.mk from ChibiOS: if verbosity then print full command only, if silent then print message only; but not both nor neither.


I use projects with make for a few years myself but only recently I discovered this trick. Sometimes I use it myself when I need to switch on/off verbosity on the fly. I hope it will be useful here as well. But if it's too hacky to accept, then I will understand.

Oh, and since there is -f flag for rm, then ignoring possible errors (such as File doesn't exist) through - prefix inside related call is not necessarily as far as I understand.

Test builds (with and without USE_VERBOSE_COMPILE=yes) have been done successfully locally on Ubuntu 18.04 with GNU Make 4.1.

One more thing. I'm aware that the original block:

ifeq ($(USE_VERBOSE_COMPILE),)
  USE_VERBOSE_COMPILE = no
endif

comes from samples with ChibiOS. But since in rules.mk there are checks for USE_VERBOSE_COMPILE = yes ? only anyway, I guess it will be more clear & less confusing to check for it here as well. But sorry in advance if I miss something here.

Now I'm very curious to hear opinions about this refactoring. :)

@bvernoux
Copy link
Member

bvernoux commented Dec 30, 2023

Thanks for your contribution that seems to be a very interesting PR to cleanup/simplify/enhance the Makefile
I will do some tests to be sure all work fine also on Windows/Linux on my side (as it seems you have tested it only on Ubuntu 18.04)

@bvernoux bvernoux merged commit f7e9dc4 into hydrabus:master Dec 30, 2023
@bvernoux
Copy link
Member

I have tested with success the build on Windows(MSYS2) & Ubuntu 22.04 LTS (with default USE_VERBOSE_COMPILE no and with export USE_VERBOSE_COMPILE=yes) and I confirm all work fine with the expected output verbose or no verbose

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

Successfully merging this pull request may close these issues.

None yet

2 participants