[edk2-devel] [PATCH V2] BaseTools:improve code to support C files with .C suffixes
Leif Lindholm
leif.lindholm at linaro.org
Wed May 8 14:26:49 UTC 2019
On Wed, May 08, 2019 at 06:59:40AM -0700, Andrew Fish wrote:
>
>
> > On May 8, 2019, at 5:08 AM, Leif Lindholm <leif.lindholm at linaro.org> wrote:
> >
> > Hi Fan Zhiju,
> >
> > I understand your reasoning, but that strengthens my view that we
> > should leave this change out.
> >
> > Actually, could we consider *dropping* support for .C files
> > altogether, since we are striving to support multiple toolchains, and
> > the two major families of toolchains we use consider .C files to be
> > fundamentally different things?
> >
> > Andrew, Laszlo, Mike?
> >
>
> Leif,
>
> I missed that ARM had extra restrictions on file extensions.
>
> [C-Code-File]
> <InputFile>
> ?.c
> ?.C
> ?.cc
> ?.CC
> ?.cpp
> ?.Cpp
> ?.CPP
>
> [C-Code-File.BASE.AARCH64,C-Code-File.SEC.AARCH64,C-Code-File.PEI_CORE.AARCH64,C-Code-File.PEIM.AARCH64,C-Code-File.BASE.ARM,C-Code-File.SEC.ARM,C-Code-File.PEI_CORE.ARM,C-Code-File.PEIM.ARM]
> <InputFile>
> ?.c
>
So did I :)
Still...
> I think there are people who try to roll there own C++ support and
> that is why the build system will pass the C++ files to the C
> compiler.
>
> At this point I think we should probably act more like a
> compiler. First add a warning, and down the road remove the support?
Yeah, that would work for me.
It's just I've had a few instances of people moving existing drivers
developed with VS (for x86) to GCC and run across issues - so getting a
warning would at least convey the fact that there *is* a portability
issue here.
Regards,
Leif
>
> Thanks,
>
> Andrew Fish
>
> > Best Regards,
> >
> > Leif
> >
> > On Wed, May 08, 2019 at 10:57:09AM +0000, Fan, ZhijuX wrote:
> >> Hi Leif,
> >>
> >> This patch is going to fix the bug in commit 05217d210e.
> >> this patch and commit 05217d210e is just for MSVC. It doesn't have any effect on GCC.
> >> this patch does not imply to compile .C as C instead of C++.
> >> We just found out that they build break because the.C file was in the source file,
> >> But the MSVC compiler allows.C files,So I fixed this bug to Change the original logic to support.C files.
> >>
> >>
> >> Any question, please let me know. Thanks.
> >>
> >> Best Regards
> >> Fan Zhiju
> >>
> >>
> >>
> >>
> >>> -----Original Message-----
> >>> From: Leif Lindholm [mailto:leif.lindholm at linaro.org]
> >>> Sent: Wednesday, May 8, 2019 5:09 PM
> >>> To: Andrew Fish <afish at apple.com>
> >>> Cc: devel at edk2.groups.io; Fan, ZhijuX <zhijux.fan at intel.com>; Gao, Liming
> >>> <liming.gao at intel.com>; Feng, Bob C <bob.c.feng at intel.com>
> >>> Subject: Re: [edk2-devel] [PATCH V2] BaseTools:improve code to support C
> >>> files with .C suffixes
> >>>
> >>> On Tue, May 07, 2019 at 07:01:26PM -0700, Andrew Fish wrote:
> >>>>
> >>>>
> >>>>> On May 7, 2019, at 7:40 AM, Leif Lindholm <leif.lindholm at linaro.org>
> >>> wrote:
> >>>>>
> >>>>> Hi Fan Zhiju,
> >>>>>
> >>>>> But where does the string come from that contains a .C suffix?
> >>>>> Is the tool internally converting things to uppercase, or is some
> >>>>> source file in the build incorrectly named?
> >>>>>
> >>>>
> >>>> Leif,
> >>>>
> >>>> Our build system defines .C as correct! I think it has been that way a very
> >>> long time.
> >>>
> >>> .C is valid, but at least for GCC it is equivalent to all of the other non-.c
> >>> options - i.e., interpret as c++. Which is why I am wondering about the case
> >>> that ends up with the build system internally processing this.
> >>>
> >>> If it is changed to permitting .C files to be compiled as C instead of
> >>> C++ (which the patch seems to imply), that sounds incorrect to me.
> >>>
> >>> /
> >>> Leif
> >>>
> >>>>
> >>> https://github.com/tianocore/edk2/blob/master/BaseTools/Conf/build_rul
> >>>> e.template#L109
> >>>>
> >>>> [C-Code-File]
> >>>> <InputFile>
> >>>> ?.c
> >>>> ?.C
> >>>> ?.cc
> >>>> ?.CC
> >>>> ?.cpp
> >>>> ?.Cpp
> >>>> ?.CPP
> >>>>
> >>>> Thanks,
> >>>>
> >>>> Andrew Fish
> >>>>
> >>>>
> >>>>> I am asking because it is not clear to me whether the patch resolves
> >>>>> a problem or hides one.
> >>>>>
> >>>>> Best Regards,
> >>>>>
> >>>>> Leif
> >>>>>
> >>>>> On Tue, May 07, 2019 at 03:05:02AM +0000, Fan, ZhijuX wrote:
> >>>>>> This problem has nothing to do with the file system, We just use
> >>>>>> the filename as a string to compare with other strings Our unittest
> >>>>>> tested minplatform, Ovmf. This problem was found when building a
> >>>>>> platform inside Intel.
> >>>>>> We've tested it on Linux and Windows.
> >>>>>>
> >>>>>> Any question, please let me know. Thanks.
> >>>>>>
> >>>>>> Best Regards
> >>>>>> Fan Zhiju
> >>>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: afish at apple.com [mailto:afish at apple.com]
> >>>>>> Sent: Tuesday, May 7, 2019 10:31 AM
> >>>>>> To: devel at edk2.groups.io; Fan, ZhijuX <zhijux.fan at intel.com>
> >>>>>> Cc: Gao, Liming <liming.gao at intel.com>; Feng, Bob C
> >>>>>> <bob.c.feng at intel.com>
> >>>>>> Subject: Re: [edk2-devel] [PATCH V2] BaseTools:improve code to
> >>>>>> support C files with .C suffixes
> >>>>>>
> >>>>>> This brings up a question? Do we tests on a file system that is case
> >>> sensitive? Is this just lack of a test case?
> >>>>>>
> >>>>>> Thanks,
> >>>>>>
> >>>>>> Andrew Fish
> >>>>>>
> >>>>>>> On May 6, 2019, at 7:22 PM, Fan, ZhijuX <zhijux.fan at intel.com> wrote:
> >>>>>>>
> >>>>>>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1773
> >>>>>>>
> >>>>>>> Build break if C file suffixes of named .C instead of .c Code not
> >>>>>>> recognize filenames with .C suffixes.
> >>>>>>>
> >>>>>>> This patch adds code to Support both .c file and .C file
> >>>>>>>
> >>>>>>> Cc: Bob Feng <bob.c.feng at intel.com>
> >>>>>>> Cc: Liming Gao <liming.gao at intel.com>
> >>>>>>> Signed-off-by: Zhiju.Fan <zhijux.fan at intel.com>
> >>>>>>> ---
> >>>>>>> BaseTools/Source/Python/AutoGen/GenMake.py | 3 ++-
> >>>>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>>>>>>
> >>>>>>> diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py
> >>>>>>> b/BaseTools/Source/Python/AutoGen/GenMake.py
> >>>>>>> index 0e0f9fd9b0..858ddedf8e 100644
> >>>>>>> --- a/BaseTools/Source/Python/AutoGen/GenMake.py
> >>>>>>> +++ b/BaseTools/Source/Python/AutoGen/GenMake.py
> >>>>>>> @@ -1035,7 +1035,8 @@ cleanlib:
> >>>>>>> CmdTargetDict[CmdSign] = "%s %s" %
> >>> (CmdTargetDict[CmdSign], SingleCommandList[-1])
> >>>>>>> Index = CommandList.index(Item)
> >>>>>>> CommandList.pop(Index)
> >>>>>>> - if SingleCommandList[-1].endswith("%s%s.c" % (TAB_SLASH,
> >>> CmdSumDict[CmdSign.lstrip('/Fo').rsplit(TAB_SLASH, 1)[0]])):
> >>>>>>> + if SingleCommandList[-1].endswith("%s%s.c" %
> >>> (TAB_SLASH, CmdSumDict[T.Target.SubDir])) or \
> >>>>>>> + SingleCommandList[-1].endswith("%s%s.C" %
> >>> (TAB_SLASH, CmdSumDict[T.Target.SubDir])):
> >>>>>>> Cpplist = CmdCppDict[T.Target.SubDir]
> >>>>>>> Cpplist.insert(0, '$(OBJLIST_%d): $(COMMON_DEPS)' %
> >>> list(self.ObjTargetDict.keys()).index(T.Target.SubDir))
> >>>>>>> T.Commands[Index] = '%s\n\t%s' % ('
> >>>>>>> \\\n\t'.join(Cpplist), CmdTargetDict[CmdSign])
> >>>>>>> --
> >>>>>>> 2.14.1.windows.1
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> <winmail.dat>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>
> >>>>>
> >>>>
> >
> >
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#40207): https://edk2.groups.io/g/devel/message/40207
Mute This Topic: https://groups.io/mt/31539232/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