Change in coreboot[master]: libpayload: Remove shell for loops in install Makefile target
Attention is currently required from: Jakub Czapiga. Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59980 )
Change subject: libpayload: Remove shell for loops in install Makefile target ......................................................................
Patch Set 2:
(5 comments)
File payloads/libpayload/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/59980/comment/4a2b8428_1ff4389a PS1, Line 107: \
I think, you forgot to remove semicolon and backslash
Done
https://review.coreboot.org/c/coreboot/+/59980/comment/6ee129aa_fb30e12f PS1, Line 107: install -m 644 $(library-targets) $(DESTDIR)/libpayload/lib/;
I think, it would be safer to change it to: […]
Done
https://review.coreboot.org/c/coreboot/+/59980/comment/a6720053_7d7aa1e2 PS1, Line 114: find include -type f -exec install -m644 {} $(DESTDIR)/libpayload/{} ;
I think there should be '*.h' wildcard filter. Otherwise it might install unnecessary files.
Only if we add such files at some point. Either we have these files during the libpayload build and we install them, or we shouldn't have them at all.
https://review.coreboot.org/c/coreboot/+/59980/comment/9cd256f2_704a284f PS1, Line 115: cd $(coreboottop)/src/commonlib/bsd && find include -type d -exec install -m755 -d $(DESTDIR)/libpayload/{} ;
Wont it create directories even if no files will be copied to them later?
These have to be created by the build system as git doesn't support empty directories. If we create directories during the libpayload build shouldn't we install them?
https://review.coreboot.org/c/coreboot/+/59980/comment/318a593a_e8b47507 PS1, Line 116: cd $(coreboottop)/src/commonlib/bsd && find include -type f -exec install -m644 {} $(DESTDIR)/libpayload/{} ;
Same as above
Ack
-
Patrick Georgi (Code Review)