[PATCH 1/1] BaseTools: INF should use latest Pcd value instead of default value

[PATCH 1/1] BaseTools: INF should use latest Pcd value instead of default value

[yi1 li]

This patch is a bug fix about FeatureFlagExpression in INF file:
 INF [Source] section now unconditionally use Pcd default value in DEC
 when handling FeatureFlagExpression, it is wrong.
 If a Pcd value has been set in the DSC file, we should use latest
 value in DSC instead of default value.

Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>

Signed-off-by: Yi Li <yi1.li@intel.com>
---
 BaseTools/Source/Python/Workspace/InfBuildData.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/Python/Workspace/InfBuildData.py b/BaseTools/Source/Python/Workspace/InfBuildData.py
index 5b9b3d7b4f..e4ff1c6686 100644
--- a/BaseTools/Source/Python/Workspace/InfBuildData.py
+++ b/BaseTools/Source/Python/Workspace/InfBuildData.py
@@ -1084,7 +1084,9 @@ class InfBuildData(ModuleBuildClassObject):
         else:
             for Name, Guid in self.Pcds:
                 if self.Pcds[(Name, Guid)].Type == 'FeatureFlag' or self.Pcds[(Name, Guid)].Type == 'FixedAtBuild':
-                    Pcds['%s.%s' % (Guid, Name)] = self.Pcds[(Name, Guid)].DefaultValue
+                    PcdFullName = '%s.%s' % (Guid, Name);
+                    if not PcdFullName in Pcds:
+                        Pcds[PcdFullName] = self.Pcds[(Name, Guid)].DefaultValue
             try:
                 Value = ValueExpression(Instance, Pcds)()
                 if Value == True:
-- 
2.31.1.windows.1

Re: [edk2-devel] [PATCH 1/1] BaseTools: INF should use latest Pcd value instead of default value

[Yuwei Chen]

Hi Yi,

Could you describe the bug with an example? In which scenario the dsc defined pcd will not override the dec definition? 

Thanks,
Christine
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of yi1 li
> Sent: Monday, July 11, 2022 1:48 PM
> To: devel@edk2.groups.io
> Cc: Li, Yi1 <yi1.li@intel.com>; Feng, Bob C <bob.c.feng@intel.com>; Gao,
> Liming <gaoliming@byosoft.com.cn>
> Subject: [edk2-devel] [PATCH 1/1] BaseTools: INF should use latest Pcd value
> instead of default value
> 
> This patch is a bug fix about FeatureFlagExpression in INF file:
>  INF [Source] section now unconditionally use Pcd default value in DEC  when
> handling FeatureFlagExpression, it is wrong.
>  If a Pcd value has been set in the DSC file, we should use latest  value in DSC
> instead of default value.
> 
> Cc: Bob Feng <bob.c.feng@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> 
> Signed-off-by: Yi Li <yi1.li@intel.com>
> ---
>  BaseTools/Source/Python/Workspace/InfBuildData.py | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/BaseTools/Source/Python/Workspace/InfBuildData.py
> b/BaseTools/Source/Python/Workspace/InfBuildData.py
> index 5b9b3d7b4f..e4ff1c6686 100644
> --- a/BaseTools/Source/Python/Workspace/InfBuildData.py
> +++ b/BaseTools/Source/Python/Workspace/InfBuildData.py
> @@ -1084,7 +1084,9 @@ class InfBuildData(ModuleBuildClassObject):
>          else:
>              for Name, Guid in self.Pcds:
>                  if self.Pcds[(Name, Guid)].Type == 'FeatureFlag' or self.Pcds[(Name,
> Guid)].Type == 'FixedAtBuild':
> -                    Pcds['%s.%s' % (Guid, Name)] = self.Pcds[(Name,
> Guid)].DefaultValue
> +                    PcdFullName = '%s.%s' % (Guid, Name);
> +                    if not PcdFullName in Pcds:
> +                        Pcds[PcdFullName] = self.Pcds[(Name,
> + Guid)].DefaultValue
>              try:
>                  Value = ValueExpression(Instance, Pcds)()
>                  if Value == True:
> --
> 2.31.1.windows.1
> 
> 
> 
> 
> 

Re: [edk2-devel] [PATCH 1/1] BaseTools: INF should use latest Pcd value instead of default value

[yi1 li]

Hi Christine,

Thanks for review, and this bug is come from commit : https://github.com/tianocore/edk2/commit/bf9230a9f3dde065c3c8b4175ccd32e44e8f0362

This commit introduce a new feature to conditional build source file in INF,
Such like: curve25519.c      |*|*|*|gEfiCryptoPkgTokenSpaceGuid.PcdOpensslEcEnabled

The issue is :
If FeatureFlagExpression is a PCD expression, not just PCD, e.g. (!gEfiTestGuid.Pcd) ,(NOT gEfiTestGuid.Pcd) ...,
Basetools can not calculate the correct value.

The root-cause is INF now use default PCD value in DEC (self.Pcds[(Name, Guid)].DefaultValue) when calculating expression,
If a PCD has be set in DSC (the new value will be stored in gPlatformFinalPcds), we need to use the new value.

Hope it's clear.

-----Original Message-----
From: Chen, Christine <yuwei.chen@intel.com> 
Sent: Friday, July 22, 2022 10:29 AM
To: devel@edk2.groups.io; Li, Yi1 <yi1.li@intel.com>
Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
Subject: RE: [edk2-devel] [PATCH 1/1] BaseTools: INF should use latest Pcd value instead of default value

Hi Yi,

Could you describe the bug with an example? In which scenario the dsc defined pcd will not override the dec definition? 

Thanks,
Christine
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of yi1 li
> Sent: Monday, July 11, 2022 1:48 PM
> To: devel@edk2.groups.io
> Cc: Li, Yi1 <yi1.li@intel.com>; Feng, Bob C <bob.c.feng@intel.com>; 
> Gao, Liming <gaoliming@byosoft.com.cn>
> Subject: [edk2-devel] [PATCH 1/1] BaseTools: INF should use latest Pcd 
> value instead of default value
> 
> This patch is a bug fix about FeatureFlagExpression in INF file:
>  INF [Source] section now unconditionally use Pcd default value in DEC  
> when handling FeatureFlagExpression, it is wrong.
>  If a Pcd value has been set in the DSC file, we should use latest  
> value in DSC instead of default value.
> 
> Cc: Bob Feng <bob.c.feng@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> 
> Signed-off-by: Yi Li <yi1.li@intel.com>
> ---
>  BaseTools/Source/Python/Workspace/InfBuildData.py | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/BaseTools/Source/Python/Workspace/InfBuildData.py
> b/BaseTools/Source/Python/Workspace/InfBuildData.py
> index 5b9b3d7b4f..e4ff1c6686 100644
> --- a/BaseTools/Source/Python/Workspace/InfBuildData.py
> +++ b/BaseTools/Source/Python/Workspace/InfBuildData.py
> @@ -1084,7 +1084,9 @@ class InfBuildData(ModuleBuildClassObject):
>          else:
>              for Name, Guid in self.Pcds:
>                  if self.Pcds[(Name, Guid)].Type == 'FeatureFlag' or 
> self.Pcds[(Name, Guid)].Type == 'FixedAtBuild':
> -                    Pcds['%s.%s' % (Guid, Name)] = self.Pcds[(Name,
> Guid)].DefaultValue
> +                    PcdFullName = '%s.%s' % (Guid, Name);
> +                    if not PcdFullName in Pcds:
> +                        Pcds[PcdFullName] = self.Pcds[(Name, 
> + Guid)].DefaultValue
>              try:
>                  Value = ValueExpression(Instance, Pcds)()
>                  if Value == True:
> --
> 2.31.1.windows.1
> 
> 
> 
> 
> 

Re: [PATCH 1/1] BaseTools: INF should use latest Pcd value instead of default value

[Bob Feng]

Reviewed-by: Bob Feng <bob.c.feng@intel.com>

-----Original Message-----
From: Li, Yi1 <yi1.li@intel.com> 
Sent: Monday, July 11, 2022 1:48 PM
To: devel@edk2.groups.io
Cc: Li, Yi1 <yi1.li@intel.com>; Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
Subject: [PATCH 1/1] BaseTools: INF should use latest Pcd value instead of default value

This patch is a bug fix about FeatureFlagExpression in INF file:
 INF [Source] section now unconditionally use Pcd default value in DEC  when handling FeatureFlagExpression, it is wrong.
 If a Pcd value has been set in the DSC file, we should use latest  value in DSC instead of default value.

Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>

Signed-off-by: Yi Li <yi1.li@intel.com>
---
 BaseTools/Source/Python/Workspace/InfBuildData.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/Python/Workspace/InfBuildData.py b/BaseTools/Source/Python/Workspace/InfBuildData.py
index 5b9b3d7b4f..e4ff1c6686 100644
--- a/BaseTools/Source/Python/Workspace/InfBuildData.py
+++ b/BaseTools/Source/Python/Workspace/InfBuildData.py
@@ -1084,7 +1084,9 @@ class InfBuildData(ModuleBuildClassObject):
         else:
             for Name, Guid in self.Pcds:
                 if self.Pcds[(Name, Guid)].Type == 'FeatureFlag' or self.Pcds[(Name, Guid)].Type == 'FixedAtBuild':
-                    Pcds['%s.%s' % (Guid, Name)] = self.Pcds[(Name, Guid)].DefaultValue
+                    PcdFullName = '%s.%s' % (Guid, Name);
+                    if not PcdFullName in Pcds:
+                        Pcds[PcdFullName] = self.Pcds[(Name, 
+ Guid)].DefaultValue
             try:
                 Value = ValueExpression(Instance, Pcds)()
                 if Value == True:
--
2.31.1.windows.1