Makefile: have HEADS_GAWK depend on locally built HEADS_MAKE #1128
Conversation
tlaurion
changed the title
|
~ @SergiiDmytruk @MrChromebox : I didn't find how to have "all" depend on "bootstrap" which would have been the cleaner option.~ EDIT: 4e57d3f works. Thanks @SergiiDmytruk for the insight! Before, we were having locally built gawk sha256sum in hashes.txt. Now we have make. Todo (make fu insights needed) @SergiiDmytruk @MrChromebox: We still have lost sha256sum of the coreboot roms in hashes.txt, which I have no clue why up until now.Any ideas? Traces:
|
Makefile
Outdated
| # Once we have a proper Make, we can just pass arguments into it | ||
| all linux cpio run: $(HEADS_MAKE) | ||
| LANG=C MAKE=$(HEADS_MAKE) $(HEADS_MAKE) $(MAKE_JOBS) $@ | ||
| LANG=C MAKE=$(HEADS_MAKE) $(MAKE_JOBS) $@ |
There was a problem hiding this comment.
Does this work? I think you might have removed wrong part of the string ($(HEADS_MAKE) instead of MAKE=$(HEADS_MAKE)).
There was a problem hiding this comment.
@SergiiDmytruk the line 94 shows LANG=C MAKE=$(HEADS_MAKE) $(MAKE_JOBS) $@
I basically changed:
LANG=C MAKE=$(HEADS_MAKE) $(HEADS_MAKE) $(MAKE_JOBS) $@
for
LANG=C MAKE=$(HEADS_MAKE) $(MAKE_JOBS) $@
Since originally there was two $(HEADS_MAKE) $(HEADS_MAKE) passed. No harm, but make logs under build/logs/* were showing that double /build/make-4.2.1/make everywhere.
There was a problem hiding this comment.
@tlaurion I probably miss something. It's just that LANG/MAKE=... sets environment variables and $(MAKE_JOBS) expands to -j... which is what should be interpreted as a command in this case, I just don't see how it can execute make at the end.
There was a problem hiding this comment.
@SergiiDmytruk : Now I see your point. Actually, I do not even understand why it was put there. Was removed under 790671f and commented at #1128 (comment)
Probably like this: bootstrap: ;
all: bootstrap
Don't see anything obvious, would need to reduce Makefile to debug this. |
@SergiiDmytruk :Some attempts. As said, this is my make fu that is probably the problem. I have to miss a concept. In master: So adding an Applying |
|
@SergiiDmytruk : So user@heads-tests:~/heads$ git diff |
tlaurion
force-pushed
the
fix_gawk_dependency_on_local_make
branch
from
9f062ae to
4e57d3f
Compare
What i'm looking for is what went wrong under 7ce12fe causing rom sha256sum output to not be part of generated hashes.txt anymore. This is now replaced by gawk/make in hashes.txt as of now and since that commit.
This is the change that was applied between parent commit c096a1f (generates rom sha256sum under hashes.txt correctly) and culprit commit 7ce12fe (which doesn't and generated gawk sha256sum instead): @SergiiDmytruk any insight on what happened to this Which for some reason doesn't apply to coreboot produced rom since that culprit commit? @SergiiDmytruk : this comment #1128 (comment) (section Traces) gives direct hashes.txt output for the same before/after commits. |
|
@SergiiDmytruk : unfortunately 4e57d3f, implementing: Also hits the So it seems that 9f062ae was a better implementation, or that |
|
@tlaurion Try making it an order-only prerequisite like this: all: | bootstrap;This way it won't appear in automatic variables |
@tlaurion Same problem, probably should be: all linux cpio run: | $(HEADS_GAWK)Mind that this prevents targets from being rebuilt when gawk is updated. Probably a better alternative for both issues would be to use normal prerequisites and specify rom file as a variable rather than abusing prerequisites: ifeq ($(CONFIG_COREBOOT), y)
all: $(build)/$(BOARD)/$(CB_OUTPUT_FILE)
else ifeq ($(CONFIG_LINUXBOOT), y)
all: $(build)/$(BOARD)/$(LB_OUTPUT_FILE)Then use it instead of |
|
As of now, I can't implement correctly any suggestions of @SergiiDmytruk, ordering not being respected. I get the same result of having only gawk attempted to be built(which should require make built first), unless 9f062ae is implemented. But we still have lost ROM hash under hashes.txt doing that. My current intuition to improve further then 9f062ae would be to move make and gawk build instructions to their individual modules files and have their install/ path hardcoded (instead of their current build/ counterpart) and passed to the rest of the builds from the global Makefile. Modules binaries and libraries are hashed and outputted under hashes.txt when deployed to their final directories (./bin ./lib etc), and for those to happen, then need to be referred in boards, added in Makefile explicit inclusion statements from boards specified variables. I think adding gawk (for libgpg-error if I recall well) was added in Makefile because it might have been the easiest thing to do, but it broke other things, like removing board final ROMs hashes under hashes.txt. Tinkering with the actual Make file makes my eyes and brain bleed and is not properly understood by anyone who seem to dare looking at it or improving it without having other things breaking. We are, as @SergiiDmytruk said, really abusing prerequisites as if now and understanding what is going on is really not easy to do, making Heads hard to maintain. I separated gawk from make version test condition since having make at version 4.2.1 was not a guarantee that gawk would also be in version 4.2.1. Then after moving away from Debian-10 to debian-11 docker image, I realized that gawk version test was flawed, and passed as good even if gawk was not installed on host. Then recently, a user reported that building from a clean commit was not working, since gawk was built without make having been locally built. This is what 9f062ae is fixing, while not fixing the whole problem. It also seems that LANG C is not filled correctly, @SergiiDmytruk is right, MAKE_OPTIONS is not filled correctly, and the path to make local binary is passed twice. This has no intended good effect since perl is rolling back to default LANG statement... It would need a rewrite by someone having the right qualifications or at least have the current process properly documented. Otherwise, its not maintained, because its not currently maintainable. |
…pected version. Maximized boards: have rom hashes outputted under hashes.txt. Still not clear why the Makefile "all" target is not doing that for each target. - Fixes a bug where "make BOARD=xyz bootstrap" was needed prior of being able to build anything else otherwise HEADS_GAWK was expecting HEADS_MAKE to be built already, which was not the case. (fixes linuxboot#1126) - As of now, I do not see any OS make/gawk being used anymore. All modules except coreboot (which uses coreboot buildstack), including musl-cross-make, are being built with local make and local gawk. Improves linuxboot#936 (comment) which showed that local make (/usb/bin/make, /usr/local/bin/make, /usr/sbin/make, /usr/local/sbin/make and /usr/bin/gawk were called locally. To redo)
tlaurion
force-pushed
the
fix_gawk_dependency_on_local_make
branch
from
4e57d3f to
790671f
Compare
|
@SergiiDmytruk @MrChromebox :
Comments? Recommendations? |
|
I think you want to add ifneq "$(HEADS_MAKE)" ""
$(HEADS_GAWK): | $(HEADS_MAKE)
endifto make local gawk actually depend on local make, otherwise they can be built in parallel.
Another cause of the trouble is that recipes are actually redefined (that I think (but not sure) it might help to split
This should help avoid redefining targets. |
…e hashes outputted into hashes.txt Makefile: make sure of precedence, even if parallelized builds of local make prior of local gawk
… not found. Weird: If we remove the target CB_OUTPUT target on maximized boards, gawk and make are built, but there are missing hashes under hashes.txt If we add CB_OUTPUT in boards, then make and gawk are not built. Here, without having time to dig that problem more, we choose to warn user to bootstrap first if there is an error, and have hashes under hashes.txt. Again folks. Your make fu is needed. I think I won't be able to bring that any further.
|
@SergiiDmytruk @MrChromebox : So I now invite the user to build bootstrap first if there is discrepancy between deployed gawk and make from OS and expected local built ones, so we keep rom hashes under hashes.txt which is now added statements in all maximized boards configs so they output the hashes themselves into hashes.txt. My attempts to separate Makefile to Makefile.inc as suggested under #1128 (comment) didn't resolve the issue. And I have spent way too much time here, I am not the right resource to resolve this issue. I am not sure I can bring this farther then c8cf23f. |
|
https://github.com/osresearch/heads/blob/182bd6bd7517c8e322bc4712db01906b41cce58c/Makefile#L580-L591 Seems to show guideline on having precedence. Why make and gawk IN the Makefile and not outside as all other modules seems to be problematic. Single ROM boards do not need to generate the checksums themselves; the ones having multiple ones need to create those hashes manually in the board config. |
|
Replaced by #1178 |
2 participants
Makefile: have HEADS_GAWK depend on locally built HEADS_MAKE if make and gawk are not found in expected 4.2.1 versions locally.
This will be the case on debian-11 and other newer OSes.
Fixes a bug where "make BOARD=xyz bootstrap" was needed prior of being able to build anything else otherwise HEADS_GAWK was expecting HEADS_MAKE to be built already, which was not the case. (fixes Error 1 and Error 127 on Build (make bootstrap needed prior of building anything else) #1126)
As of now, I do not see any OS make/gawk being used anymore. All modules except coreboot (which uses coreboot buildstack), including musl-cross-make, are being built with local make and local gawk. Improves Reproducibility: Track external commands used #936 (comment) which showed that local make (/usb/bin/make, /usr/local/bin/make, /usr/sbin/make, /usr/local/sbin/make and /usr/bin/gawk were called locally. To restest)