On Wed, May 08, 2019 at 06:59:40AM -0700, Andrew Fish wrote: > > > > On May 8, 2019, at 5:08 AM, Leif Lindholm <leif.lindh...@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.lindh...@linaro.org] > >>> Sent: Wednesday, May 8, 2019 5:09 PM > >>> To: Andrew Fish <af...@apple.com> > >>> Cc: devel@edk2.groups.io; Fan, ZhijuX <zhijux....@intel.com>; Gao, Liming > >>> <liming....@intel.com>; Feng, Bob C <bob.c.f...@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.lindh...@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: af...@apple.com [mailto:af...@apple.com] > >>>>>> Sent: Tuesday, May 7, 2019 10:31 AM > >>>>>> To: devel@edk2.groups.io; Fan, ZhijuX <zhijux....@intel.com> > >>>>>> Cc: Gao, Liming <liming....@intel.com>; Feng, Bob C > >>>>>> <bob.c.f...@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....@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.f...@intel.com> > >>>>>>> Cc: Liming Gao <liming....@intel.com> > >>>>>>> Signed-off-by: Zhiju.Fan <zhijux....@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/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-