-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
OvmfPkg, ArmVirtPkg: Fix unable to build with -D NETWORK_ENABLE=0 #6087
Conversation
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. |
ffa225d
to
a276474
Compare
Updated. |
…t are disabled Avoid including some network components when most are disabled with -D NETWORK_ENABLE=0 This issue showed up when addressing https://bugzilla.tianocore.org/show_bug.cgi?id=4829 in tianocore#6087 . Commit 7f17a15 partially applies this change; this rolls it out to other places where it applies. Signed-off-by: Mike Beaton [email protected]
…ETWORK_ENABLE=0 This issue showed up when addressing https://bugzilla.tianocore.org/show_bug.cgi?id=4829 in tianocore#6087 . Commit 7f17a15 partially applies this change; this rolls it out to other places where it applies. Signed-off-by: Mike Beaton [email protected]
…ABLE=0 This issue showed up when addressing https://bugzilla.tianocore.org/show_bug.cgi?id=4829 in tianocore#6087 . Commit 7f17a15 partially applies this change; this rolls it out to other places where it applies. Signed-off-by: Mike Beaton [email protected]
…ABLE=0 This issue showed up when addressing https://bugzilla.tianocore.org/show_bug.cgi?id=4829 in tianocore#6087 . Commit 7f17a15 partially applies this change; this rolls it out to other places where it applies. Signed-off-by: Mike Beaton [email protected]
…ABLE=0 This issue showed up when addressing https://bugzilla.tianocore.org/show_bug.cgi?id=4829 in tianocore#6087 . Commit 7f17a15 partially applies this change; this rolls it out to other places where it applies. Signed-off-by: Mike Beaton <[email protected]>
…ABLE=0 This issue showed up when addressing https://bugzilla.tianocore.org/show_bug.cgi?id=4829 in tianocore#6087 . Various OvmfPkg and ArmVirtPkg platforms include some residual NetworkPkg components when compiled with -D NETWORK_ENABLE=0, even though they use NetworkPkg includes intended to allow all NetworkPkg components to be disabled on this flag. For the OvmfPkg Intel platforms only, commit d933ec1 started the change of not including these residual NetworkPkg component, and commit 7f17a15 completed it. This commit rolls these changes out to the other packages where they make sense in the same way. Signed-off-by: Mike Beaton <[email protected]>
Is this PR waiting for anything? |
On the review of a NetworkPkg maintainer afaict. |
|
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! |
This issue showed up when addressing https://bugzilla.tianocore.org/show_bug.cgi?id=4829 in tianocore#6087 . Various OvmfPkg and ArmVirtPkg platforms include some residual NetworkPkg components when compiled with -D NETWORK_ENABLE=0, even though they use NetworkPkg includes intended to allow all NetworkPkg components to be disabled on this flag. As part of a sequence of three commits to address this, this commit to NetworkPkg is not strictly necessary in order to ensure that no NetworkPkg components are included, but it does make it easier to confirm this, by ensuring that no NetworkPkg .inf files are even accessed when building with -D NETWORK_ENABLE=0. (For example with this commit, but not without it, we can destructively rename all instances of the string "NetworkPkg.dec" in the entire project, and confirm that we can stil build with -D NETWORK_ENABLE=0.) Signed-off-by: Mike Beaton <[email protected]>
This issue showed up when addressing https://bugzilla.tianocore.org/show_bug.cgi?id=4829 in tianocore#6087 . Various OvmfPkg and ArmVirtPkg platforms include some residual NetworkPkg components when compiled with -D NETWORK_ENABLE=0, even though they use NetworkPkg includes intended to allow all NetworkPkg components to be disabled on this flag. For the OvmfPkg Intel platforms only, commit d933ec1 started the change of not including these residual NetworkPkg component, and commit 7f17a15 completed it. This commit rolls these changes out to the remaining OvmfPkg platforms where they make sense in the same way. Signed-off-by: Mike Beaton <[email protected]>
This issue showed up when addressing https://bugzilla.tianocore.org/show_bug.cgi?id=4829 in tianocore#6087 . Various OvmfPkg and ArmVirtPkg platforms include some residual NetworkPkg components when compiled with -D NETWORK_ENABLE=0, even though they use NetworkPkg includes intended to allow all NetworkPkg components to be disabled on this flag. For the OvmfPkg Intel platforms only, commit d933ec1 started the change of not including these residual NetworkPkg component, and commit 7f17a15 completed it. This commit rolls these changes out to the ArmVirtPkg platforms where they make sense in the same way. Signed-off-by: Mike Beaton <[email protected]>
This issue showed up when addressing https://bugzilla.tianocore.org/show_bug.cgi?id=4829 in tianocore#6087 . Various OvmfPkg and ArmVirtPkg platforms include some residual NetworkPkg components when compiled with -D NETWORK_ENABLE=0, even though they use NetworkPkg includes intended to allow all NetworkPkg components to be disabled on this flag. As part of a sequence of three commits to address this, this commit to NetworkPkg is not strictly necessary in order to ensure that no NetworkPkg components are included, but it does make it easier to confirm this, by ensuring that no NetworkPkg .inf files are even accessed when building with -D NETWORK_ENABLE=0. (For example with this commit, but not without it, we can destructively rename all instances of the string "NetworkPkg.dec" in the entire project, and confirm that we can still build with -D NETWORK_ENABLE=0.) Signed-off-by: Mike Beaton <[email protected]>
This issue showed up when addressing https://bugzilla.tianocore.org/show_bug.cgi?id=4829 in tianocore#6087 . Various OvmfPkg and ArmVirtPkg platforms include some residual NetworkPkg components when compiled with -D NETWORK_ENABLE=0, even though they use NetworkPkg includes intended to allow all NetworkPkg components to be disabled on this flag. For the OvmfPkg Intel platforms only, commit d933ec1 started the change of not including these residual NetworkPkg component, and commit 7f17a15 completed it. This commit rolls these changes out to the remaining OvmfPkg platforms where they make sense in the same way. Signed-off-by: Mike Beaton <[email protected]>
This issue showed up when addressing https://bugzilla.tianocore.org/show_bug.cgi?id=4829 in tianocore#6087 . Various OvmfPkg and ArmVirtPkg platforms include some residual NetworkPkg components when compiled with -D NETWORK_ENABLE=0, even though they use NetworkPkg includes intended to allow all NetworkPkg components to be disabled on this flag. For the OvmfPkg Intel platforms only, commit d933ec1 started the change of not including these residual NetworkPkg components, and commit 7f17a15 completed it. This commit rolls these changes out to the remaining OvmfPkg platforms where they make sense in the same way. Signed-off-by: Mike Beaton <[email protected]>
This issue showed up when addressing https://bugzilla.tianocore.org/show_bug.cgi?id=4829 in tianocore#6087 . Various OvmfPkg and ArmVirtPkg platforms include some residual NetworkPkg components when compiled with -D NETWORK_ENABLE=0, even though they use NetworkPkg includes intended to allow all NetworkPkg components to be disabled on this flag. For the OvmfPkg Intel platforms only, commit d933ec1 started the change of not including these residual NetworkPkg components, and commit 7f17a15 completed it. This commit rolls these changes out to the ArmVirtPkg platforms where they make sense in the same way. Signed-off-by: Mike Beaton <[email protected]>
This issue showed up when addressing https://bugzilla.tianocore.org/show_bug.cgi?id=4829 in tianocore#6087 . Various OvmfPkg and ArmVirtPkg platforms include some residual NetworkPkg components when compiled with -D NETWORK_ENABLE=0, even though they use NetworkPkg includes intended to allow all NetworkPkg components to be disabled on this flag. For the OvmfPkg Intel platforms only, commit d933ec1 started the change of not including these residual NetworkPkg components, and commit 7f17a15 completed it. This commit rolls these changes out to the remaining OvmfPkg platforms where they make sense in the same way. Signed-off-by: Mike Beaton <[email protected]> ArmVirtPkg: Include no network components with -D NETWORK_ENABLE=0 This issue showed up when addressing https://bugzilla.tianocore.org/show_bug.cgi?id=4829 in tianocore#6087 . Various OvmfPkg and ArmVirtPkg platforms include some residual NetworkPkg components when compiled with -D NETWORK_ENABLE=0, even though they use NetworkPkg includes intended to allow all NetworkPkg components to be disabled on this flag. For the OvmfPkg Intel platforms only, commit d933ec1 started the change of not including these residual NetworkPkg components, and commit 7f17a15 completed it. This commit rolls these changes out to the ArmVirtPkg platforms where they make sense in the same way. Signed-off-by: Mike Beaton <[email protected]> OvmfPkg: Use OvmfPkg/Include/*/Shell*.inc throughout While fixing (tianocore#6092) the fact that some OvmfPkg and ArmVirtPkg platforms included residual NetworkPkg components even when compiled with -D NETWORK_ENABLE=0, it was noted that OvmfPkg/Include/*/Shell*.inc files which already applied this fix logic are available and already used in some OvmfPkg platforms. This commit applies these files consistently within OvmfPkg. This has the side effects that: - Some platforms now include one or more of HttpDynamicCommand, VariablePolicyDynamicCommand and LinuxInitrdDynamicShellCommand when they previously did not - Whether the UEFI Shell is included (by ShellDxe.fdf.inc) depends on whether Secure Boot is enabled (on those platforms which support enabling secure boot) To address the second issue, in this commit we add ShellDefines.dsc.inc, which defaults BUILD_SHELL to FALSE when SECURE_BOOT_ENABLE is TRUE and vice versa, and then modify ShellDxe.fdf.inc so that it conditionally includes shell components only depending on BUILD_SHELL. We additionally enable UEFI Shell in the CI matrix for those tests which rely on UEFI Shell being present with Secure Boot enabled. Signed-off-by: Mike Beaton <[email protected]>
https://bugzilla.tianocore.org/show_bug.cgi?id=4829 7f17a15 (2024/02/22) "OvmfPkg: Shell*.inc: allow building without network support" breaks building OVMF with `-D NETWORK_ENABLE=0`. Before this commit we could build OVMF e.g. with the following command in the OvmfPkg directory: ./build.sh -D NETWORK_ENABLE=0 After the commit the same command fails early with: /home/user/OpenSource/edk2/OvmfPkg/OvmfPkgX64.dsc(15): error F001: Pcd (gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections) defined in DSC is not declared in DEC files referenced in INF files in FDF. Arch: ['X64'] This commit conditionally removes the undefined Pcd reference in NetworkPkg which is part of this issue. Similar changes are needed in separate commits for OvmfPkg (and for ArmVirtPkg, since the issue also exists there, although masked by another issue). Signed-off-by: Mike Beaton <[email protected]>
https://bugzilla.tianocore.org/show_bug.cgi?id=4829 7f17a15 (2024/02/22) "OvmfPkg: Shell*.inc: allow building without network support" breaks building OVMF with `-D NETWORK_ENABLE=0`. Before this commit we could build OVMF e.g. with the following command in the OvmfPkg directory: ./build.sh -D NETWORK_ENABLE=0 After the commit the same command fails early with: /home/user/OpenSource/edk2/OvmfPkg/OvmfPkgX64.dsc(15): error F001: Pcd (gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections) defined in DSC is not declared in DEC files referenced in INF files in FDF. Arch: ['X64'] The problem applies in Intel OvmfPkg platforms. Additionally, it applies in various other OvmfPkg platforms, but is masked buy another issue; namely that these platforms incorrectly still include some network packages when most are disabled. (A fix for that issue has previously been made, in OvmfPkg Intel platforms only, by d933ec1 followed by 7f17a15 .) This commit conditionally removes the undefined Pcd references in all OvmfPkg platforms which are now affected by this issue, and in all those which would be affected as and when the other issue mentioned above is fixed. Signed-off-by: Mike Beaton <[email protected]>
https://bugzilla.tianocore.org/show_bug.cgi?id=4829 7f17a15 (2024/02/22) "OvmfPkg: Shell*.inc: allow building without network support" breaks building OVMF with `-D NETWORK_ENABLE=0`. Before this commit we could build OVMF e.g. with the following command in the OvmfPkg directory: ./build.sh -D NETWORK_ENABLE=0 After the commit the same command fails early with: /home/user/OpenSource/edk2/OvmfPkg/OvmfPkgX64.dsc(15): error F001: Pcd (gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections) defined in DSC is not declared in DEC files referenced in INF files in FDF. Arch: ['X64'] This problem also applies in the ArmVirtPkg platforms which are modified here, but is currently masked by another issue, namely that these platforms incorrectly still include some network packages when most are disabled. (A fix for this was previously applied, for OvmfPkg Intel platforms only, by d933ec1 followed by 7f17a15 .) This commit was created at the same time as the commits resolving this issue in NetworkPkg and OvmfPkg. It makes conditional the Pcd references in ArmVirtPkg platforms which will become references to undefined Pcds as and when the other issue mentioned above is fixed. Signed-off-by: Mike Beaton <[email protected]>
dece51e
to
1e9c407
Compare
⚠ 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: Attn Admins: Admin Instructions:
|
⚠ 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: Attn Admins: Admin Instructions:
|
This issue showed up when addressing https://bugzilla.tianocore.org/show_bug.cgi?id=4829 in tianocore#6087 . Various OvmfPkg and ArmVirtPkg platforms include some residual NetworkPkg components when compiled with -D NETWORK_ENABLE=0, even though they use NetworkPkg includes intended to allow all NetworkPkg components to be disabled on this flag. For the OvmfPkg Intel platforms only, commit d933ec1 started the change of not including these residual NetworkPkg components, and commit 7f17a15 completed it. This commit rolls these changes out to the remaining OvmfPkg platforms where they make sense in the same way. Signed-off-by: Mike Beaton <[email protected]>
This issue showed up when addressing https://bugzilla.tianocore.org/show_bug.cgi?id=4829 in tianocore#6087 . Various OvmfPkg and ArmVirtPkg platforms include some residual NetworkPkg components when compiled with -D NETWORK_ENABLE=0, even though they use NetworkPkg includes intended to allow all NetworkPkg components to be disabled on this flag. For the OvmfPkg Intel platforms only, commit d933ec1 started the change of not including these residual NetworkPkg components, and commit 7f17a15 completed it. This commit rolls these changes out to the ArmVirtPkg platforms where they make sense in the same way. Signed-off-by: Mike Beaton <[email protected]>
This issue showed up when addressing https://bugzilla.tianocore.org/show_bug.cgi?id=4829 in tianocore#6087 . Various OvmfPkg and ArmVirtPkg platforms include some residual NetworkPkg components when compiled with -D NETWORK_ENABLE=0, even though they use NetworkPkg includes intended to allow all NetworkPkg components to be disabled on this flag. For the OvmfPkg Intel platforms only, commit d933ec1 started the change of not including these residual NetworkPkg components, and commit 7f17a15 completed it. This commit rolls these changes out to the remaining OvmfPkg platforms where they make sense in the same way. Signed-off-by: Mike Beaton <[email protected]>
This issue showed up when addressing https://bugzilla.tianocore.org/show_bug.cgi?id=4829 in tianocore#6087 . Various OvmfPkg and ArmVirtPkg platforms include some residual NetworkPkg components when compiled with -D NETWORK_ENABLE=0, even though they use NetworkPkg includes intended to allow all NetworkPkg components to be disabled on this flag. For the OvmfPkg Intel platforms only, commit d933ec1 started the change of not including these residual NetworkPkg components, and commit 7f17a15 completed it. This commit rolls these changes out to the ArmVirtPkg platforms where they make sense in the same way. Signed-off-by: Mike Beaton <[email protected]>
This issue showed up when addressing https://bugzilla.tianocore.org/show_bug.cgi?id=4829 in tianocore#6087 . Various OvmfPkg and ArmVirtPkg platforms include some residual NetworkPkg components when compiled with -D NETWORK_ENABLE=0, even though they use NetworkPkg includes intended to allow all NetworkPkg components to be disabled on this flag. For the OvmfPkg Intel platforms only, commit d933ec1 started the change of not including these residual NetworkPkg components, and commit 7f17a15 completed it. This commit rolls these changes out to the remaining OvmfPkg platforms where they make sense in the same way. Signed-off-by: Mike Beaton <[email protected]>
This issue showed up when addressing https://bugzilla.tianocore.org/show_bug.cgi?id=4829 in tianocore#6087 . Various OvmfPkg and ArmVirtPkg platforms include some residual NetworkPkg components when compiled with -D NETWORK_ENABLE=0, even though they use NetworkPkg includes intended to allow all NetworkPkg components to be disabled on this flag. For the OvmfPkg Intel platforms only, commit d933ec1 started the change of not including these residual NetworkPkg components, and commit 7f17a15 completed it. This commit rolls these changes out to the ArmVirtPkg platforms where they make sense in the same way. Signed-off-by: Mike Beaton <[email protected]>
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 7f17a15 . 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 #6092 is applied.
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 #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 #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.