diff mbox series

[2/3] ovmf: drop gcc-12 specific BUILD_CFLAGS setting

Message ID 20260511092935.2631121-3-joaomarcos.costa@bootlin.com
State Under Review
Headers show
Series ovmf: add some rework to the recipe | expand

Commit Message

Joao Marcos Costa May 11, 2026, 9:29 a.m. UTC
This was handled upstream a while ago:

https://github.com/tianocore/edk2/blob/master/BaseTools/Source/C/DevicePath/GNUmakefile#L27

Signed-off-by: João Marcos Costa <joaomarcos.costa@bootlin.com>
---
 meta/recipes-core/ovmf/ovmf_git.bb | 4 ----
 1 file changed, 4 deletions(-)

Comments

Quentin Schulz May 11, 2026, 1:27 p.m. UTC | #1
Hi João,

On 5/11/26 11:29 AM, João Marcos Costa wrote:
> This was handled upstream a while ago:
> 
> https://github.com/tianocore/edk2/blob/master/BaseTools/Source/C/DevicePath/GNUmakefile#L27
> 

It's not because upstream supports it that the version we're building 
has it, so pointing at a commit and mentioning it's part of the version 
we're building would be nice.

22130dcd98b4 ("Basetools: turn off gcc12 warning") part of 
edk2-stable202205 tag (and also edk2-stable202511 which we currently are at.

Also, unclear whether this single file is the only place this flag being 
set would be enough as the flag was set globally and now it's only in a 
specific file within edk2 source code.

Cheers,
Quentin
Joao Marcos Costa May 11, 2026, 1:40 p.m. UTC | #2
Hello,

On 5/11/26 15:27, Quentin Schulz via lists.openembedded.org wrote:
> Hi João,
> 
> On 5/11/26 11:29 AM, João Marcos Costa wrote:
>> This was handled upstream a while ago:
>>
>> https://github.com/tianocore/edk2/blob/master/BaseTools/Source/C/DevicePath/GNUmakefile#L27
>>
> 
> It's not because upstream supports it that the version we're building 
> has it, so pointing at a commit and mentioning it's part of the version 
> we're building would be nice.
> 
> 22130dcd98b4 ("Basetools: turn off gcc12 warning") part of 
> edk2-stable202205 tag (and also edk2-stable202511 which we currently are 
> at.

That's very much true. I will stick with this approach for the next time.

> Also, unclear whether this single file is the only place this flag being 
> set would be enough as the flag was set globally and now it's only in a 
> specific file within edk2 source code.
> 
> Cheers,
> Quentin

In any case, the warnings/errors would have been noticed during the 
build if this flag was still needed (and missing).
Quentin Schulz May 11, 2026, 1:52 p.m. UTC | #3
Hi João,

On 5/11/26 3:40 PM, Joao Marcos Costa wrote:
> Hello,
> 
> On 5/11/26 15:27, Quentin Schulz via lists.openembedded.org wrote:
>> Hi João,
>>
>> On 5/11/26 11:29 AM, João Marcos Costa wrote:
>>> This was handled upstream a while ago:
>>>
>>> https://eur02.safelinks.protection.outlook.com/? 
>>> url=https%3A%2F%2Fgithub.com%2Ftianocore%2Fedk2%2Fblob%2Fmaster%2FBaseTools%2FSource%2FC%2FDevicePath%2FGNUmakefile%23L27&data=05%7C02%7Cquentin.schulz%40cherry.de%7Cd69acb4606d94a43ff0b08deaf62e1b9%7C5e0e1b5221b54e7b83bb514ec460677e%7C0%7C0%7C639141036395687030%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=D5wD6%2Bvq%2BYJP3wUROtdEswwwc4RJFYmYvt7GLzsvJbo%3D&reserved=0
>>>
>>
>> It's not because upstream supports it that the version we're building 
>> has it, so pointing at a commit and mentioning it's part of the 
>> version we're building would be nice.
>>
>> 22130dcd98b4 ("Basetools: turn off gcc12 warning") part of edk2- 
>> stable202205 tag (and also edk2-stable202511 which we currently are at.
> 
> That's very much true. I will stick with this approach for the next time.
> 
>> Also, unclear whether this single file is the only place this flag 
>> being set would be enough as the flag was set globally and now it's 
>> only in a specific file within edk2 source code.
>>
>> Cheers,
>> Quentin
> 
> In any case, the warnings/errors would have been noticed during the 
> build if this flag was still needed (and missing).
> 

Not necessarily true as I don't think we build GCC for native recipes 
(and ovmf has a native variant), so it depends on the GCC on the host 
(unless you have uninative?). I don't know the autobuilder 
infrastructure, e.g. whether we build patches on multiple workers on 
different distros to have a big GCC version test matrix.

But yes, building with a newer GCC will generally not make warnings from 
previous versions disappear and I'm hoping upstream has had time to fix 
warnings returned by GCC12 in the last 4 years :)

Cheers,
Quentin
Joao Marcos Costa May 11, 2026, 2 p.m. UTC | #4
Hello,

On 5/11/26 15:52, Quentin Schulz via lists.openembedded.org wrote:
(...)
> 
> Not necessarily true as I don't think we build GCC for native recipes 
> (and ovmf has a native variant), so it depends on the GCC on the host 
> (unless you have uninative?). I don't know the autobuilder 
> infrastructure, e.g. whether we build patches on multiple workers on 
> different distros to have a big GCC version test matrix.
> 
> But yes, building with a newer GCC will generally not make warnings from 
> previous versions disappear and I'm hoping upstream has had time to fix 
> warnings returned by GCC12 in the last 4 years :)
> 
> Cheers,
> Quentin

Yep, my whole point is based on the the distros in SANITY_TESTED_DISTROS 
being recent enough to ship GCC > 12 :)

I should have made it explicit in my commit message.
Richard Purdie May 12, 2026, 8:46 a.m. UTC | #5
On Mon, 2026-05-11 at 16:00 +0200, Joao Marcos Costa via lists.openembedded.org wrote:
> Hello,
> 
> On 5/11/26 15:52, Quentin Schulz via lists.openembedded.org wrote:
> (...)
> > 
> > Not necessarily true as I don't think we build GCC for native recipes 
> > (and ovmf has a native variant), so it depends on the GCC on the host 
> > (unless you have uninative?). I don't know the autobuilder 
> > infrastructure, e.g. whether we build patches on multiple workers on 
> > different distros to have a big GCC version test matrix.
> > 
> > But yes, building with a newer GCC will generally not make warnings from 
> > previous versions disappear and I'm hoping upstream has had time to fix 
> > warnings returned by GCC12 in the last 4 years :)
> > 
> > Cheers,
> > Quentin
> 
> Yep, my whole point is based on the the distros in SANITY_TESTED_DISTROS 
> being recent enough to ship GCC > 12 :)
> 
> I should have made it explicit in my commit message.

I hate to say this but:

Alma/Centos/Rocky 8 have gcc 8
Debian 11 has gcc 10

and we don't use buildtools on debian 11.

I'm therefore not sure how you concluded that from SANITY_TESTED_DISTROS.

Cheers,

Richard
Joao Marcos Costa May 12, 2026, 9:02 a.m. UTC | #6
Hello,

On 5/12/26 10:46, Richard Purdie wrote:
> On Mon, 2026-05-11 at 16:00 +0200, Joao Marcos Costa via lists.openembedded.org wrote:
>> Hello,
>>
>> On 5/11/26 15:52, Quentin Schulz via lists.openembedded.org wrote:
>> (...)
>>>
>>> Not necessarily true as I don't think we build GCC for native recipes
>>> (and ovmf has a native variant), so it depends on the GCC on the host
>>> (unless you have uninative?). I don't know the autobuilder
>>> infrastructure, e.g. whether we build patches on multiple workers on
>>> different distros to have a big GCC version test matrix.
>>>
>>> But yes, building with a newer GCC will generally not make warnings from
>>> previous versions disappear and I'm hoping upstream has had time to fix
>>> warnings returned by GCC12 in the last 4 years :)
>>>
>>> Cheers,
>>> Quentin
>>
>> Yep, my whole point is based on the the distros in SANITY_TESTED_DISTROS
>> being recent enough to ship GCC > 12 :)
>>
>> I should have made it explicit in my commit message.
> 
> I hate to say this but:
> 
> Alma/Centos/Rocky 8 have gcc 8
> Debian 11 has gcc 10
> 
> and we don't use buildtools on debian 11.
> 
> I'm therefore not sure how you concluded that from SANITY_TESTED_DISTROS.
> 
> Cheers,
> 
> Richard

Ah, my bad...

 From this list here:
https://docs.yoctoproject.org/ref-manual/system-requirements.html#supported-linux-distributions

Considering the versions of Fedora and Ubuntu, I presumed the other 
distros were at least as recent (in terms of packages versions) as 
Fedora 39 (which ships GCC 13).

I will check the results in SWAT to see if this warnings pops up, but 
the commit message could use a rewording to address this point and also 
some questions raised by Quentin, so a v2 will be sent later in this week.
Richard Purdie May 12, 2026, 9:06 a.m. UTC | #7
On Tue, 2026-05-12 at 11:02 +0200, Joao Marcos Costa wrote:
> Hello,
> 
> On 5/12/26 10:46, Richard Purdie wrote:
> > On Mon, 2026-05-11 at 16:00 +0200, Joao Marcos Costa via lists.openembedded.org wrote:
> > > Hello,
> > > 
> > > On 5/11/26 15:52, Quentin Schulz via lists.openembedded.org wrote:
> > > (...)
> > > > 
> > > > Not necessarily true as I don't think we build GCC for native recipes
> > > > (and ovmf has a native variant), so it depends on the GCC on the host
> > > > (unless you have uninative?). I don't know the autobuilder
> > > > infrastructure, e.g. whether we build patches on multiple workers on
> > > > different distros to have a big GCC version test matrix.
> > > > 
> > > > But yes, building with a newer GCC will generally not make warnings from
> > > > previous versions disappear and I'm hoping upstream has had time to fix
> > > > warnings returned by GCC12 in the last 4 years :)
> > > > 
> > > > Cheers,
> > > > Quentin
> > > 
> > > Yep, my whole point is based on the the distros in SANITY_TESTED_DISTROS
> > > being recent enough to ship GCC > 12 :)
> > > 
> > > I should have made it explicit in my commit message.
> > 
> > I hate to say this but:
> > 
> > Alma/Centos/Rocky 8 have gcc 8
> > Debian 11 has gcc 10
> > 
> > and we don't use buildtools on debian 11.
> > 
> > I'm therefore not sure how you concluded that from SANITY_TESTED_DISTROS.
> > 
> > Cheers,
> > 
> > Richard
> 
> Ah, my bad...
> 
>  From this list here:
> https://docs.yoctoproject.org/ref-manual/system-requirements.html#supported-linux-distributions
> 
> Considering the versions of Fedora and Ubuntu, I presumed the other 
> distros were at least as recent (in terms of packages versions) as 
> Fedora 39 (which ships GCC 13).
> 
> I will check the results in SWAT to see if this warnings pops up, but
> the commit message could use a rewording to address this point and also 
> some questions raised by Quentin, so a v2 will be sent later in this week.

We currently support distros with gcc 10, I think that is clear. 

It may or may not show up on the autobuilder due to sstate reuse, so
relying on that as a way to check this is not appropriate.

Also note:

https://docs.yoctoproject.org/dev/ref-manual/system-requirements.html#required-git-tar-python-make-and-gcc-versions

Personally I'd love to remove this but we simply can't without changing
our documented host requirements.

Cheers,

Richard
Joao Marcos Costa May 12, 2026, 9:17 a.m. UTC | #8
Hello,

On 5/12/26 11:06, Richard Purdie wrote:
> On Tue, 2026-05-12 at 11:02 +0200, Joao Marcos Costa wrote:
>> Hello,
>>
>> On 5/12/26 10:46, Richard Purdie wrote:
>>> On Mon, 2026-05-11 at 16:00 +0200, Joao Marcos Costa via lists.openembedded.org wrote:
>>>> Hello,
>>>>
>>>> On 5/11/26 15:52, Quentin Schulz via lists.openembedded.org wrote:
>>>> (...)
>>>>>
>>>>> Not necessarily true as I don't think we build GCC for native recipes
>>>>> (and ovmf has a native variant), so it depends on the GCC on the host
>>>>> (unless you have uninative?). I don't know the autobuilder
>>>>> infrastructure, e.g. whether we build patches on multiple workers on
>>>>> different distros to have a big GCC version test matrix.
>>>>>
>>>>> But yes, building with a newer GCC will generally not make warnings from
>>>>> previous versions disappear and I'm hoping upstream has had time to fix
>>>>> warnings returned by GCC12 in the last 4 years :)
>>>>>
>>>>> Cheers,
>>>>> Quentin
>>>>
>>>> Yep, my whole point is based on the the distros in SANITY_TESTED_DISTROS
>>>> being recent enough to ship GCC > 12 :)
>>>>
>>>> I should have made it explicit in my commit message.
>>>
>>> I hate to say this but:
>>>
>>> Alma/Centos/Rocky 8 have gcc 8
>>> Debian 11 has gcc 10
>>>
>>> and we don't use buildtools on debian 11.
>>>
>>> I'm therefore not sure how you concluded that from SANITY_TESTED_DISTROS.
>>>
>>> Cheers,
>>>
>>> Richard
>>
>> Ah, my bad...
>>
>>   From this list here:
>> https://docs.yoctoproject.org/ref-manual/system-requirements.html#supported-linux-distributions
>>
>> Considering the versions of Fedora and Ubuntu, I presumed the other
>> distros were at least as recent (in terms of packages versions) as
>> Fedora 39 (which ships GCC 13).
>>
>> I will check the results in SWAT to see if this warnings pops up, but
>> the commit message could use a rewording to address this point and also
>> some questions raised by Quentin, so a v2 will be sent later in this week.
> 
> We currently support distros with gcc 10, I think that is clear.

It is clear, yes, but my point here is the -Wno-error=stringop-overflow 
already being available upstream in the version of edk2 we currently 
use, and - as per what I understood in their git history - the flag was 
added for the very same reason we have it in the recipe.

> 
> It may or may not show up on the autobuilder due to sstate reuse, so
> relying on that as a way to check this is not appropriate.
> 
> Also note:
> 
> https://docs.yoctoproject.org/dev/ref-manual/system-requirements.html#required-git-tar-python-make-and-gcc-versions
> 
> Personally I'd love to remove this but we simply can't without changing
> our documented host requirements.
> 
> Cheers,
> 
> Richard
> 
> 
>
Richard Purdie May 12, 2026, 9:51 a.m. UTC | #9
On Tue, 2026-05-12 at 11:17 +0200, Joao Marcos Costa wrote:
> Hello,
> 
> On 5/12/26 11:06, Richard Purdie wrote:
> > On Tue, 2026-05-12 at 11:02 +0200, Joao Marcos Costa wrote:
> > > Hello,
> > > 
> > > On 5/12/26 10:46, Richard Purdie wrote:
> > > > On Mon, 2026-05-11 at 16:00 +0200, Joao Marcos Costa via
> > > > lists.openembedded.org wrote:
> > > > > Hello,
> > > > > 
> > > > > On 5/11/26 15:52, Quentin Schulz via lists.openembedded.org
> > > > > wrote:
> > > > > (...)
> > > > > > 
> > > > > > Not necessarily true as I don't think we build GCC for
> > > > > > native recipes
> > > > > > (and ovmf has a native variant), so it depends on the GCC
> > > > > > on the host
> > > > > > (unless you have uninative?). I don't know the autobuilder
> > > > > > infrastructure, e.g. whether we build patches on multiple
> > > > > > workers on
> > > > > > different distros to have a big GCC version test matrix.
> > > > > > 
> > > > > > But yes, building with a newer GCC will generally not make
> > > > > > warnings from
> > > > > > previous versions disappear and I'm hoping upstream has had
> > > > > > time to fix
> > > > > > warnings returned by GCC12 in the last 4 years :)
> > > > > > 
> > > > > > Cheers,
> > > > > > Quentin
> > > > > 
> > > > > Yep, my whole point is based on the the distros in
> > > > > SANITY_TESTED_DISTROS
> > > > > being recent enough to ship GCC > 12 :)
> > > > > 
> > > > > I should have made it explicit in my commit message.
> > > > 
> > > > I hate to say this but:
> > > > 
> > > > Alma/Centos/Rocky 8 have gcc 8
> > > > Debian 11 has gcc 10
> > > > 
> > > > and we don't use buildtools on debian 11.
> > > > 
> > > > I'm therefore not sure how you concluded that from
> > > > SANITY_TESTED_DISTROS.
> > > > 
> > > > Cheers,
> > > > 
> > > > Richard
> > > 
> > > Ah, my bad...
> > > 
> > >   From this list here:
> > > https://docs.yoctoproject.org/ref-manual/system-requirements.html#supported-linux-distributions
> > > 
> > > Considering the versions of Fedora and Ubuntu, I presumed the
> > > other
> > > distros were at least as recent (in terms of packages versions)
> > > as
> > > Fedora 39 (which ships GCC 13).
> > > 
> > > I will check the results in SWAT to see if this warnings pops up,
> > > but
> > > the commit message could use a rewording to address this point
> > > and also
> > > some questions raised by Quentin, so a v2 will be sent later in
> > > this week.
> > 
> > We currently support distros with gcc 10, I think that is clear.
> 
> It is clear, yes, but my point here is the -Wno-error=stringop-
> overflow 
> already being available upstream in the version of edk2 we currently 
> use, and - as per what I understood in their git history - the flag
> was 
> added for the very same reason we have it in the recipe.

Ok, that makes sense. 

Cheers,

Richard
diff mbox series

Patch

diff --git a/meta/recipes-core/ovmf/ovmf_git.bb b/meta/recipes-core/ovmf/ovmf_git.bb
index 779e9f8196..8e2d172f66 100644
--- a/meta/recipes-core/ovmf/ovmf_git.bb
+++ b/meta/recipes-core/ovmf/ovmf_git.bb
@@ -14,10 +14,6 @@  PACKAGECONFIG[debug] = ",,,"
 PACKAGECONFIG[secureboot] = ",,,"
 PACKAGECONFIG[tpm] = "-D TPM_ENABLE=TRUE,-D TPM_ENABLE=FALSE,,"
 
-# GCC12 trips on it
-#see https://src.fedoraproject.org/rpms/edk2/blob/rawhide/f/0032-Basetools-turn-off-gcc12-warning.patch
-BUILD_CFLAGS += "-Wno-error=stringop-overflow"
-
 SRC_URI = "gitsm://github.com/tianocore/edk2.git;branch=master;protocol=https;tag=${PV} \
            file://0001-ovmf-update-path-to-native-BaseTools.patch \
            file://0002-BaseTools-makefile-adjust-to-build-in-under-bitbake.patch \