edk2
edk2 copied to clipboard
tianocore
OvmfPkg, ArmVirtPkg: Fix unable to build with -D NETWORK_ENABLE=0
Description
This PR fixes https://bugzilla.tianocore.org/show_bug.cgi?id=4829 , namely the issue that Intel OvmfPkg platforms can no longer be built with -D NETWORK_ENABLE=0 after 7f17a155640a2a9e1f7b0f3522628ee2c6f62624 . In addition it fixes the same issue in non-Intel OvmfPkg platforms and ArmVirtPkg platforms, even though the issue in those other platforms is masked until https://github.com/tianocore/edk2/pull/6092 is applied.
- [ ] Breaking change? NO
- [ ] Impacts security? NO
- [ ] Includes tests? NO
- Perhaps ideally this would update the CI to build all these platforms also with -D NETWORK_ENABLE=0, but I have not (at least so far) done this. (Note that everything built in CI with -D NETWORK_ENABLE=1 (default) (which is some but I am not sure if all of the platforms I have changed) still passes.)
How This Was Tested
Confirm that after these changes Intel OvmfPkg variants build and run without network support, when compiled with -D NETWORK_ENABLE=0. Confirm that when paired with https://github.com/tianocore/edk2/pull/6092 the other affected packages build correctly with -D NETWORK_ENABLE=0. Confirm that everything still builds and runs successfully in CI.
Integration Instructions
Paired PR https://github.com/tianocore/edk2/pull/6092 addresses the issue discovered while preparing this, namely that other OvmfPkg platforms and ArmVirtPkg platforms are only not showing this same issue because it is masked in those cases by the fact that those packages are still incorrectly including some NetworkPkg components when they should be disabled.
Please change the title so it describes the change being made. The link to the bugzilla can go into the PR description and the commit message
Please change the title so it describes the change being made. The link to the bugzilla can go into the PR description and the commit message
I can happily do that.
Looking at the various .dsc packages this PR changes, not all of them complain even when built without this fix. I was still investigating that, hence the initial preliminary status of the PR.
It turns out that the packages showing that behaviour include bits of NetworkPkg even when NETWORK_ENABLE is FALSE. They probably shouldn't, and if they'd didn't they would need this fix, but I think that's a place for possible future useful work, not a problem with this patch.
While I briefly have the floor, can I ask another quick question: I've put a few previous PRs to edk2 here on GitHub and had zero response - hence I really was just expecting to check the CI, and not see any other feedback! I do, sometimes (!), get a response to email patches sent to the edk2-devel list, and was expecting to need to send this patch there, once I was happy with it. Has there been any change in process recently, making it so that GitHub PRs are now allowed as well as (or even preferred instead of) email list patches? Thanks in advance, and apologies for missing whatever announcements I may have missed, about this.
Yes, edk2 has switched to a PR-based workflow. So please fill title + description etc. Or create them as 'draft' PRs if all you want is run CI.
https://github.com/tianocore/edk2/pull/6092 ~includes this same commit, but also~ fixes the other related issue.
I have updated to add this PR to apply to LoongArchVirtQemu platform as well, since it is also affected as discovered while preparing https://github.com/tianocore/edk2/pull/6092.
⚠ WARNING: Cannot add some reviewers: A user specified as a reviewer for this PR is not a collaborator of the repository. Please add them as a collaborator to the repository so they can be requested in the future.
Non-collaborators requested:
- @bibo-mao
- @lixianglai
Attn Admins:
- @jljusten
- @lgao4
- @mdkinney
- @vincent-j-zimmer
- @jkmathews
- @miki-intel-work
Admin Instructions:
- Add the non-collaborators as collaborators to the appropriate team(s) listed in teams
- If they are no longer needed as reviewers, remove them from
Maintainers.txt
![tianocore-assign-reviewers[bot] avatar](https://avatars.githubusercontent.com/u/352162?v=4 "Published by a pub.dev verified publisher"){kind=small}
⚠ WARNING: Cannot add some reviewers: A user specified as a reviewer for this PR is not a collaborator of the repository. Please add them as a collaborator to the repository so they can be requested in the future.
Non-collaborators requested:
- @bibo-mao
- @lixianglai
Attn Admins:
- @jljusten
- @lgao4
- @mdkinney
- @vincent-j-zimmer
- @jkmathews
- @miki-intel-work
Admin Instructions:
- Add the non-collaborators as collaborators to the appropriate team(s) listed in teams
- If they are no longer needed as reviewers, remove them from
Maintainers.txt
⚠ WARNING: Cannot add some reviewers: A user specified as a reviewer for this PR is not a collaborator of the repository. Please add them as a collaborator to the repository so they can be requested in the future.
Non-collaborators requested:
- @bibo-mao
- @lixianglai
Attn Admins:
- @jljusten
- @lgao4
- @mdkinney
- @vincent-j-zimmer
- @jkmathews
- @miki-intel-work
Admin Instructions:
- Add the non-collaborators as collaborators to the appropriate team(s) listed in teams
- If they are no longer needed as reviewers, remove them from
Maintainers.txt
⚠ WARNING: Cannot add some reviewers: A user specified as a reviewer for this PR is not a collaborator of the repository. Please add them as a collaborator to the repository so they can be requested in the future.
Non-collaborators requested:
- @bibo-mao
- @lixianglai
Attn Admins:
- @jljusten
- @lgao4
- @mdkinney
- @vincent-j-zimmer
- @jkmathews
- @miki-intel-work
Admin Instructions:
- Add the non-collaborators as collaborators to the appropriate team(s) listed in teams
- If they are no longer needed as reviewers, remove them from
Maintainers.txt
⚠ WARNING: Cannot add some reviewers: A user specified as a reviewer for this PR is not a collaborator of the repository. Please add them as a collaborator to the repository so they can be requested in the future.
Non-collaborators requested:
- @bibo-mao
- @lixianglai
Attn Admins:
- @jljusten
- @lgao4
- @mdkinney
- @vincent-j-zimmer
- @jkmathews
- @miki-intel-work
Admin Instructions:
- Add the non-collaborators as collaborators to the appropriate team(s) listed in teams
- If they are no longer needed as reviewers, remove them from
Maintainers.txt
⚠ WARNING: Cannot add some reviewers: A user specified as a reviewer for this PR is not a collaborator of the repository. Please add them as a collaborator to the repository so they can be requested in the future.
Non-collaborators requested:
- @bibo-mao
- @lixianglai
Attn Admins:
- @jljusten
- @lgao4
- @mdkinney
- @vincent-j-zimmer
- @jkmathews
- @miki-intel-work
Admin Instructions:
- Add the non-collaborators as collaborators to the appropriate team(s) listed in teams
- If they are no longer needed as reviewers, remove them from
Maintainers.txt
Please fix the failed PatchCheck build.
@SaloniKasbekar
I've done that.
Apart from one overlong line (an error message, which was previously verbatim, now split), I also had to split this into three commits in order to pass the patch check. This seems a bit artificial - was it wanted or should it remain one commit?
Splitting to multiple commits does not play well with GitHub in that ironically GitHub only runs CI on the tip commit of a PR. So in this case patch check is not automatically run on two out of the three resultant commits. I worked round this by pushing up the commits to tip one at a time. (cc @kraxel)
⚠ WARNING: Cannot add some reviewers: A user specified as a reviewer for this PR is not a collaborator of the repository. Please add them as a collaborator to the repository so they can be requested in the future.
Non-collaborators requested:
- @bibo-mao
- @lixianglai
Attn Admins:
- @jljusten
- @lgao4
- @mdkinney
- @vincent-j-zimmer
- @jkmathews
- @miki-intel-work
Admin Instructions:
- Add the non-collaborators as collaborators to the appropriate team(s) listed in teams
- If they are no longer needed as reviewers, remove them from
Maintainers.txt
Is this PR waiting for anything?
On the review of a NetworkPkg maintainer afaict.
Is this PR waiting for anything?
On the review of a NetworkPkg maintainer afaict.
@SaloniKasbekar @Zclarkwilliams ?
Apologies I asked @SaloniKasbekar and @weltling for re-review after they had already approved, after realising that the issue applied in LoongArchVirtQemu and updating the PR to apply fix there too - with hindsight I should probably have left their green ticks as they were!
EDIT: And I managed to post this comment exactly as @SaloniKasbekar was re-approving anyway. :-/ Many thanks!
⚠ WARNING: Cannot add some reviewers: A user specified as a reviewer for this PR is not a collaborator of the repository. Please add them as a collaborator to the repository so they can be requested in the future.
Non-collaborators requested:
- @bibo-mao
- @lixianglai
Attn Admins:
- @jljusten
- @lgao4
- @mdkinney
- @vincent-j-zimmer
- @jkmathews
- @miki-intel-work
Admin Instructions:
- Add the non-collaborators as collaborators to the appropriate team(s) listed in teams
- If they are no longer needed as reviewers, remove them from
Maintainers.txt
⚠ WARNING: Cannot add some reviewers: A user specified as a reviewer for this PR is not a collaborator of the repository. Please add them as a collaborator to the repository so they can be requested in the future.
Non-collaborators requested:
- @bibo-mao
- @lixianglai
Attn Admins:
- @jljusten
- @lgao4
- @mdkinney
- @vincent-j-zimmer
- @jkmathews
- @miki-intel-work
Admin Instructions:
- Add the non-collaborators as collaborators to the appropriate team(s) listed in teams
- If they are no longer needed as reviewers, remove them from
Maintainers.txt