Attention is currently required from: Elyes Haouas, Martin L Roth, Matt DeVillier.
Julius Werner has posted comments on this change by Martin L Roth. ( https://review.coreboot.org/c/coreboot/+/81026?usp=email )
Change subject: Makefiles: Download 3rdparty/chromeec when needed ......................................................................
Patch Set 4:
(4 comments)
File .gitignore:
https://review.coreboot.org/c/coreboot/+/81026/comment/720c837f_190f6691?usp... : PS4, Line 15: 3rdparty I think we should probably separate this into its own directory (`3rdparty-local`?) because otherwise it may be confusing which things in 3rdparty are tracked through the submodule system and which are ignored.
File 3rdparty/Makefile.mk:
https://review.coreboot.org/c/coreboot/+/81026/comment/3f2a9a35_a5a1c286?usp... : PS4, Line 4: $(CONFIG_EC_GOOGLE_CHROMEEC_FIRMWARE_BUILTIN), Is this the right check? Isn't `$(CONFIG_EC_GOOGLE_CHROMEEC_FIRMWARE_BUILTIN)` always either `y` or `n` (i.e. not empty)?
https://review.coreboot.org/c/coreboot/+/81026/comment/6c941dfe_916d528f?usp... : PS4, Line 9: CONFIG_CHROMEEC_COMMIT := e486b388a7 Shouldn't this hash just be the Kconfig default value? (Also, you should be adding those new options to Kconfig with this same patch.)
https://review.coreboot.org/c/coreboot/+/81026/comment/fa3b55dd_cfe68913?usp... : PS4, Line 15: CHROMEEC_GIT_TASK += $(shell cd $(CHROMEEC_DIR) && git checkout $(CONFIG_CHROMEEC_COMMIT) 2>&1) Shouldn't we do the checkout in both cases (even if the directory exists, in case the current hash changed)? Otherwise, I'm not quite sure why you fetch if you don't checkout.