public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: "Gao, Liming" <liming.gao@intel.com>
Cc: edk2-devel-01 <edk2-devel@lists.01.org>
Subject: Re: [PATCH 0/6] BaseTools/Source/C: take EXTRA_OPTFLAGS and EXTRA_LDFLAGS from the caller
Date: Mon, 6 Aug 2018 18:41:24 +0200	[thread overview]
Message-ID: <c1f5468f-8ac2-79c3-d3fe-fa35a98cdad9@redhat.com> (raw)
In-Reply-To: <44ddfa8b-7af5-47a7-d584-6777213706e3@redhat.com>

Hi Liming,

On 08/06/18 17:18, Laszlo Ersek wrote:
> On 08/06/18 16:48, Gao, Liming wrote:
>> Laszlo:
>>   Thanks for your detail information. I understand EXTRA_OPTFLAGS.
>>   So, its name is OK to me.
>>
>>   On Pccts, it is the third party code. I would like to make the
>>   minimal change. So, I ask whether we not touch it.
>
> OK, thank you, I'll look into that.

I started writing up a summary for the stake-holders of
<https://bugzilla.redhat.com/show_bug.cgi?id=1540244>, explaining that
some source code that goes into the VfrCompile utility is native to the
edk2 project, while the code that *generates* the lexer and parser
source code for VfrCompile comes from the PCCTS project, and is used
only temporarily. And, that this should be a good enough reason to
ignore PCCTS, because in upstream the maintainers prefer not touching
PCCTS source.

However: our git history for "BaseTools/Source/C/VfrCompile/Pccts" does
not corroborate this preference.

Consider:

(a)  1  30fdf1140b8d [2009-07-17] Check In tool source code based on Build tool project revision r1655.
     2  b69fd59e6f1a [2014-08-25] Fix nmake cleanall bugs.
     3  5ddccf34c4f5 [2015-07-08] BaseTools: Fix build on FreeBSD and allow use of non-gcc system compiler
     4  819a2394f17f [2016-01-11] BaseTools/VfrCompile: honor CC if it is set
     5  4ac14ceae076 [2016-09-08] BaseTools VfrCompile Pccts: Update GCC Flags to the specific one with BUILD_ prefix

    Commits #3 through #5 modify the same set of files as my patches 4-6
    -- the "antlr" and "dlg" makefiles.

(b)  6  99e55970ff07 [2016-10-20] BaseTools: Fix typos in comments and variables

    This is from Gary's series

      [edk2] [PATCH 00/33] Fix typos in comments and variables

    and it modifies "dlg" source code.

(c)  7  bab5ad2fd14b [2016-11-08] BaseTools/VfrCompile: Add checks for array access
     8  77dee0b1859d [2016-11-08] BaseTools/VfrCompile: Avoid freeing freed memory in classes
     9  d55638362727 [2016-11-08] BaseTools/VfrCompile/Pccts: Add virtual destructor for class DLGInputStream
    10  fef15ecd20dd [2016-11-08] BaseTools/VfrCompile/Pccts: Make assignment operator not returning void

    These four commits (#7 through #10) are from Hao's series

      [edk2] [PATCH v2 00/53] Resolve issues for C source codes in BaseTools

    and they modify PCCTS headers.

(d) 11  5b26adf03a0b [2016-12-20] BaseTools: fix format-security build warnings
    12  8230d45bba51 [2016-12-20] BaseTools: fix format type build warnings

    Commits #11 and #12 are from Heyi's series

      [edk2] [PATCH 0/4] Fix GCC build warnings for BaseTools

    and they modify the "antlr" source code.

(e) 13  0a64f49fde09 [2016-12-23] BaseTools/Pccts: Resolve GCC sting format mismatch build warning

    This patch is again from Hao, and it modifies utility code in PCCTS
    that is built into both "dlg" and "antlr" (namely, "set.c").

(f) 14  a5b84d3480b4 [2018-01-02] BaseTools: eliminate unused expression result
    15  4e97974c1e52 [2018-01-02] BaseTools: silence parentheses-equality warning

    These are from a series that Zenith432 posted without a cover
    letter. They modify "antlr" and "dlg" source code.

The above examples imply that we have modified both the makefiles and
the source code under PCCTS, over time.

Do you still prefer that I drop those parts of my series?

I can attempt to do that, but then I cannot tell the RHBZ#1540244
stakeholders that we "generally" avoid patching the bundled PCCTS
instance -- because, we do patch it whenever necessary.

Thanks!
Laszlo


  reply	other threads:[~2018-08-06 16:41 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-26  0:44 [PATCH 0/6] BaseTools/Source/C: take EXTRA_OPTFLAGS and EXTRA_LDFLAGS from the caller Laszlo Ersek
2018-07-26  0:44 ` [PATCH 1/6] BaseTools/footer.makefile: expand BUILD_CFLAGS last for C files too Laszlo Ersek
2018-07-26  0:44 ` [PATCH 2/6] BaseTools/header.makefile: remove "-c" from BUILD_CFLAGS Laszlo Ersek
2018-07-26  0:44 ` [PATCH 3/6] BaseTools/Source/C: split "-O2" to BUILD_OPTFLAGS Laszlo Ersek
2018-07-26  0:44 ` [PATCH 4/6] BaseTools/Pccts: clean up antlr and dlg makefiles Laszlo Ersek
2018-07-26  0:44 ` [PATCH 5/6] BaseTools/Source/C: take EXTRA_OPTFLAGS from the caller Laszlo Ersek
2018-07-26  0:44 ` [PATCH 6/6] BaseTools/Source/C: take EXTRA_LDFLAGS " Laszlo Ersek
2018-08-02 15:40 ` [PATCH 0/6] BaseTools/Source/C: take EXTRA_OPTFLAGS and " Gao, Liming
2018-08-02 17:40   ` Laszlo Ersek
2018-08-06 14:48     ` Gao, Liming
2018-08-06 15:18       ` Laszlo Ersek
2018-08-06 16:41         ` Laszlo Ersek [this message]
2018-08-07  5:27           ` Gao, Liming
2018-08-07 12:23             ` Laszlo Ersek
2018-08-08 21:15               ` Laszlo Ersek

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=c1f5468f-8ac2-79c3-d3fe-fa35a98cdad9@redhat.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