Attention is currently required from: Nico Huber, Martin L Roth, Michał Kotyla, Paul Menzel.
Maciej Pijanowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/70102 )
Change subject: util/ectool/Makefile: improve cross-compilation support ......................................................................
Patch Set 2:
(2 comments)
File util/ectool/Makefile:
https://review.coreboot.org/c/coreboot/+/70102/comment/7cd90d53_6dbe8f15 PS1, Line 3: CC ?= gcc
It's common practice to honor the environment variables. IIRC, GNU even […]
It is typical for cross-compilation environments to expose these via environment variables. See e.g. https://danyspin97.org/blog/makefiles-best-practices/ (Compiler section)
File util/ectool/Makefile:
https://review.coreboot.org/c/coreboot/+/70102/comment/7458e7c7_a2944d76 PS2, Line 12: LDFLAGS ?= -lioperm
This doesn't seem like good practice. `ioperm` is needed. Why should the […]
See for example here: https://sourceforge.net/p/dos2unix/dos2unix/ci/master/tree/dos2unix/Makefile... for a nice example of having LDFLAGS_USER allowing to inject additional flags from environment, keeping the required default libraries in the Makefile.
We should also allow for providing the value of OS_ARCH from environment. Using host tools to determine the target environment is not a good idea. The same file also can be used as an example here: https://sourceforge.net/p/dos2unix/dos2unix/ci/master/tree/dos2unix/Makefile...
-
Maciej Pijanowski (Code Review)