[edk2-devel] [Patch] BaseTools/DscBuildData: Fix PCD autogen include file conflict

Michael D Kinney michael.d.kinney at intel.com
Tue Feb 4 20:27:49 UTC 2020


Liming,

I have entered the following BZ:

https://bugzilla.tianocore.org/show_bug.cgi?id=2503

Mike

> -----Original Message-----
> From: Gao, Liming <liming.gao at intel.com>
> Sent: Monday, February 3, 2020 10:45 PM
> To: Kinney, Michael D <michael.d.kinney at intel.com>;
> devel at edk2.groups.io
> Cc: Feng, Bob C <bob.c.feng at intel.com>; Gao, Liming
> <liming.gao at intel.com>
> Subject: RE: [Patch] BaseTools/DscBuildData: Fix PCD
> autogen include file conflict
> 
> Mike:
>   If we document this assumption in build
> specification, I agree this patch. Can you enter one BZ
> for build spec update?
> 
>   For this patch, Reviewed-by: Liming Gao
> <liming.gao at intel.com>
> 
> Thanks
> Liming
> > -----Original Message-----
> > From: Kinney, Michael D <michael.d.kinney at intel.com>
> > Sent: Tuesday, February 4, 2020 1:52 AM
> > To: Gao, Liming <liming.gao at intel.com>;
> devel at edk2.groups.io; Kinney, Michael D
> <michael.d.kinney at intel.com>
> > Cc: Feng, Bob C <bob.c.feng at intel.com>
> > Subject: RE: [Patch] BaseTools/DscBuildData: Fix PCD
> autogen include file conflict
> >
> > Liming,
> >
> > Yes.  I think that is a reasonable assumption.
> >
> > We can document that restriction in the EDK II Build
> Specification
> > that standard POSIX include files (e.g. <stdio.h>)
> must never be
> > placed in the same directory with other include files
> that are
> > required for Structured PCD data types or
> defines/enums that may
> > be used to set Structured PCD field values.  We can
> enter a
> > Tianocore BZ to update the specifications.
> >
> > Thanks,
> >
> > Mike
> >
> > > -----Original Message-----
> > > From: Gao, Liming <liming.gao at intel.com>
> > > Sent: Sunday, February 2, 2020 11:33 PM
> > > To: Kinney, Michael D <michael.d.kinney at intel.com>;
> > > devel at edk2.groups.io
> > > Cc: Feng, Bob C <bob.c.feng at intel.com>
> > > Subject: RE: [Patch] BaseTools/DscBuildData: Fix
> PCD
> > > autogen include file conflict
> > >
> > > Mike:
> > >   Yes. Build tool needs to know host OS behavior
> and
> > > get the full include path. For this patch, it skips
> > > some include paths. If this include path also
> includes
> > > other required header file, it will cause the build
> > > break. Can we have the assumption that all sys
> header
> > > files and other non-sys header files are not in the
> > > same include directory?
> > >
> > > Thanks
> > > Liming
> > > > -----Original Message-----
> > > > From: Kinney, Michael D
> <michael.d.kinney at intel.com>
> > > > Sent: Monday, February 3, 2020 2:40 PM
> > > > To: Gao, Liming <liming.gao at intel.com>;
> > > devel at edk2.groups.io; Kinney, Michael D
> > > <michael.d.kinney at intel.com>
> > > > Cc: Feng, Bob C <bob.c.feng at intel.com>
> > > > Subject: RE: [Patch] BaseTools/DscBuildData: Fix
> PCD
> > > autogen include file conflict
> > > >
> > > > Liming,
> > > >
> > > > The build tools would need to know which env var
> to
> > > query for
> > > > all OS/host tool chain combinations and how to
> parse
> > > that
> > > > information for full paths in an OS specific
> manner.
> > > We should
> > > > not build that type of information into the build
> > > tools.
> > > >
> > > > The fix I have provided does not need this
> > > information.
> > > >
> > > > Mike
> > > >
> > > > > -----Original Message-----
> > > > > From: Gao, Liming <liming.gao at intel.com>
> > > > > Sent: Sunday, February 2, 2020 4:59 PM
> > > > > To: Kinney, Michael D
> <michael.d.kinney at intel.com>;
> > > > > devel at edk2.groups.io
> > > > > Cc: Feng, Bob C <bob.c.feng at intel.com>
> > > > > Subject: RE: [Patch] BaseTools/DscBuildData:
> Fix
> > > PCD
> > > > > autogen include file conflict
> > > > >
> > > > > Mike:
> > > > >   Can we consider to parse INCLUDE env value
> and
> > > add
> > > > > those path to -I options as the first priority?
> > > > >
> > > > > Thanks
> > > > > Liming
> > > > > > -----Original Message-----
> > > > > > From: Kinney, Michael D
> > > <michael.d.kinney at intel.com>
> > > > > > Sent: Thursday, January 30, 2020 8:46 AM
> > > > > > To: devel at edk2.groups.io
> > > > > > Cc: Feng, Bob C <bob.c.feng at intel.com>; Gao,
> > > Liming
> > > > > <liming.gao at intel.com>
> > > > > > Subject: [Patch] BaseTools/DscBuildData: Fix
> PCD
> > > > > autogen include file conflict
> > > > > >
> > > > > >
> > > https://bugzilla.tianocore.org/show_bug.cgi?id=2494
> > > > > >
> > > > > > When using structured PCDs, a C application
> is
> > > auto
> > > > > generated
> > > > > > to fill in the structured PCD value.  The C
> > > > > application uses
> > > > > > the standard include files <stdio.h>,
> <stdlib.h>,
> > > and
> > > > > <string.h>.
> > > > > > This C application also supports include
> paths
> > > from
> > > > > package DEC
> > > > > > files when a structured PCD declaration
> provides
> > > a
> > > > > <Packages>
> > > > > > list.  The complete list of include paths are
> -I
> > > > > options for
> > > > > > include paths from package DEC files and the
> > > > > compiler's standard
> > > > > > include paths.
> > > > > >
> > > > > > -I include paths are higher priority than the
> > > > > standard include
> > > > > > paths.  If the -I included paths from package
> DEC
> > > > > files contain
> > > > > > <stdio.h>, <stdlib.h>, or <string.h> the
> wrong
> > > > > include files are
> > > > > > used to compile the C application for the
> > > structured
> > > > > PCD value.
> > > > > >
> > > > > > Update GenerateByteArrayValue() to skip a
> package
> > > DEC
> > > > > include
> > > > > > paths that contain <stdio.h>, <stdlib.h>, or
> > > > > <string.h>.
> > > > > >
> > > > > > Build failures were observed when adding a
> > > structured
> > > > > PCD to
> > > > > > CryptoPkg.  CryptoPkg contains <stdio.h>,
> > > <stdlib.h>,
> > > > > and
> > > > > > <string.h> in the path
> CryptoPkg/Library/Include
> > > to
> > > > > support
> > > > > > building Open SSL.  The Library/Include path
> is
> > > > > listed as a
> > > > > > private include path in CryptoPkg.dec.
> Without
> > > this
> > > > > change, the
> > > > > > standard include files designed to support
> build
> > > > > OpenSLL are
> > > > > > used to build the structured PCD C
> application,
> > > and
> > > > > that build
> > > > > > fails.
> > > > > >
> > > > > > Other packages that provide a standard C lib
> or a
> > > > > gasket for
> > > > > > a subset of the standard C lib will run into
> this
> > > > > same issue
> > > > > > if they also define and use a Structured PCD.
> So
> > > > > this issue
> > > > > > is not limited to the CryptoPkg.
> > > > > >
> > > > > > Cc: Bob Feng <bob.c.feng at intel.com>
> > > > > > Cc: Liming Gao <liming.gao at intel.com>
> > > > > > Signed-off-by: Michael D Kinney
> > > > > <michael.d.kinney at intel.com>
> > > > > > ---
> > > > > >  .../Source/Python/Workspace/DscBuildData.py
> |
> > > 18
> > > > > +++++++++++++++++-
> > > > > >  1 file changed, 17 insertions(+), 1
> deletion(-)
> > > > > >
> > > > > > diff --git
> > > > >
> a/BaseTools/Source/Python/Workspace/DscBuildData.py
> > > > >
> b/BaseTools/Source/Python/Workspace/DscBuildData.py
> > > > > > index c65a0dd346..be6688dc75 100644
> > > > > > ---
> > > > >
> a/BaseTools/Source/Python/Workspace/DscBuildData.py
> > > > > > +++
> > > > >
> b/BaseTools/Source/Python/Workspace/DscBuildData.py
> > > > > > @@ -1,7 +1,7 @@
> > > > > >  ## @file
> > > > > >  # This file is used to create a database
> used by
> > > > > build tool
> > > > > >  #
> > > > > > -# Copyright (c) 2008 - 2019, Intel
> Corporation.
> > > All
> > > > > rights reserved.<BR>
> > > > > > +# Copyright (c) 2008 - 2020, Intel
> Corporation.
> > > All
> > > > > rights reserved.<BR>
> > > > > >  # (C) Copyright 2016 Hewlett Packard
> Enterprise
> > > > > Development LP<BR>
> > > > > >  # SPDX-License-Identifier: BSD-2-Clause-
> Patent
> > > > > >  #
> > > > > > @@ -2667,6 +2667,22 @@ class
> > > > > DscBuildData(PlatformBuildClassObject):
> > > > > >              for pkg in PcdDependDEC:
> > > > > >                  if pkg in PlatformInc:
> > > > > >                      for inc in
> PlatformInc[pkg]:
> > > > > > +                        #
> > > > > > +                        # Get list of files
> in
> > > > > potential -I include path
> > > > > > +                        #
> > > > > > +                        FileList =
> os.listdir
> > > > > (str(inc))
> > > > > > +                        #
> > > > > > +                        # Skip -I include
> path
> > > if
> > > > > one of the include files required
> > > > > > +                        # by PcdValueInit.c
> are
> > > > > present in the include paths from
> > > > > > +                        # the DEC file.
> > > > > PcdValueInit.c must use the standard include
> > > > > > +                        # files from the
> host
> > > > > compiler.
> > > > > > +                        #
> > > > > > +                        if 'stdio.h' in
> > > FileList:
> > > > > > +                          continue
> > > > > > +                        if 'stdlib.h' in
> > > FileList:
> > > > > > +                          continue
> > > > > > +                        if 'string.h' in
> > > FileList:
> > > > > > +                          continue
> > > > > >                          MakeApp += '-I'  +
> > > str(inc)
> > > > > + ' '
> > > > > >
> > > IncSearchList.append(inc)
> > > > > >          MakeApp = MakeApp + '\n'
> > > > > > --
> > > > > > 2.21.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53739): https://edk2.groups.io/g/devel/message/53739
Mute This Topic: https://groups.io/mt/70261740/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-





More information about the edk2-devel-archive mailing list