Having an undocumented (or silently installed, therefore unexpected) dependency is undesirable (especially for a firmware), no question about that.
Sorry, I still get the impression that there is a fundamental misunderstanding about what Git submodules are here.
I know how submodules work, I believe Nico too. I also know the internal git data model very well.
Sorry, there were multiple streams of discussion in here, I just want to make sure we're all on the same page on what it actually is. I felt the characterization of "silently installed" that I quoted made it sound like something else.
the Git SHA of the submodule HEAD is stored in the main coreboot repo.
My argument is solely on complexity, but please don't trust that hash too much.
I was trying to say that it is just as secure and trustworthy as code stored in the main Git repository. I don't think we need to get into the details of SHA1 collision resistance here, it's just how Git works right now for everything it does (submodules and normal files). I just wanted to clarify that there should be no security (or availability) concerns with this "dependency" because that was mentioned somewhere above.
Sure, but more source code and in particular across more repositories is a lot more complexity than less source code in a single repository.
I mean, the specific functions that cbfstool uses from vboot are not very complex at all (and since it's a submodule they're right there, you can just look at them, they're just stored under 3rdparty/ instead of under src/). And the main reason we're doing this is to avoid reimplementing the same thing in coreboot again, which would add a lot more complexity to builds that are using vboot for verification -- not only would you have the same algorithm twice, there would also likely be API differences (e.g. how hashes are handled or hash types are encoded) that then require complicated translation whenever coreboot and vboot parts try to talk to each other. You can basically: a) have a coreboot hash API and a vboot hash API, and coreboot code calling into vboot will need to do a lot of translation, or b) have a coreboot hash API for other stuff but use the vboot API for vboot-specific coreboot code, meaning that we'd confusingly use two separate hash APIs within coreboot (and sometimes they may still need to interwork and require translation), or c) just make the vboot API available to all of coreboot and use it everywhere. I really think that last one ends up being the least complex and confusing, overall.
And if they do, and you can tell me what it is, we are happy to apply the same standard to vboot going forward.
I think that's a fantastic promise! I also understand and agree with your request for standardization/documentation, something to set expectations, that's 100% reasonable.
Like I said in my first mail here, I'm really not trying to make this hard for you! I'm really trying to keep things smooth and problem-free both for the people who do and especially also for those who don't want to use vboot stuff. Just please let me find other ways to do that rather than having to hide every small use of utility functions behind a hard switch to turn it off, because in practice that leads to a lot of detail problems that make things very hard to implement and maintain. We should be able to integrate this such that it will just build and link quietly in the background without causing anyone any troubles, same as all of the other support code that's in the main coreboot repository. This thread was the first time I heard that the __attribute__((fallthrough)) is causing a lot of people pain, and that's an absolute non-issue to fix... just give me a day or two and it'll be gone. If there are more issues causing pain with any specific compiler or build environment, let me know and I'll try to help.