public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Liming Gao" <liming.gao@intel.com>
To: "Kinney, Michael D" <michael.d.kinney@intel.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Feng, Bob C" <bob.c.feng@intel.com>
Subject: Re: [Patch] BaseTools/DscBuildData: Fix PCD autogen include file conflict
Date: Mon, 3 Feb 2020 07:33:03 +0000	[thread overview]
Message-ID: <a12d741e36ff492ab9cdbc813c692970@intel.com> (raw)
In-Reply-To: <E92EE9817A31E24EB0585FDF735412F5B9E85634@ORSMSX113.amr.corp.intel.com>

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@intel.com>
> Sent: Monday, February 3, 2020 2:40 PM
> To: Gao, Liming <liming.gao@intel.com>; devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: Feng, Bob C <bob.c.feng@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@intel.com>
> > Sent: Sunday, February 2, 2020 4:59 PM
> > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > devel@edk2.groups.io
> > Cc: Feng, Bob C <bob.c.feng@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@intel.com>
> > > Sent: Thursday, January 30, 2020 8:46 AM
> > > To: devel@edk2.groups.io
> > > Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming
> > <liming.gao@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@intel.com>
> > > Cc: Liming Gao <liming.gao@intel.com>
> > > Signed-off-by: Michael D Kinney
> > <michael.d.kinney@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


  reply	other threads:[~2020-02-03  7:33 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-30  0:46 [Patch] BaseTools/DscBuildData: Fix PCD autogen include file conflict Michael D Kinney
2020-02-03  0:59 ` Liming Gao
2020-02-03  6:39   ` Michael D Kinney
2020-02-03  7:33     ` Liming Gao [this message]
2020-02-03 17:52       ` Michael D Kinney
2020-02-04  6:45         ` Liming Gao
2020-02-04 20:27           ` Michael D Kinney
2020-02-03  2:45 ` Liming Gao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a12d741e36ff492ab9cdbc813c692970@intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox