From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 167BA211B112A for ; Mon, 28 Jan 2019 05:48:30 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id CFB218831F; Mon, 28 Jan 2019 13:48:29 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-224.rdu2.redhat.com [10.10.120.224]) by smtp.corp.redhat.com (Postfix) with ESMTP id 71B4B5C21D; Mon, 28 Jan 2019 13:48:28 +0000 (UTC) To: "Feng, Bob C" Cc: "edk2-devel@lists.01.org" , "Gao, Liming" References: <20190125045626.14700-1-bob.c.feng@intel.com> <64c49419-fe14-1be4-bfaf-1d601aedb294@redhat.com> <08650203BA1BD64D8AD9B6D5D74A85D160071956@SHSMSX101.ccr.corp.intel.com> <08650203BA1BD64D8AD9B6D5D74A85D160076553@SHSMSX101.ccr.corp.intel.com> From: Laszlo Ersek Message-ID: Date: Mon, 28 Jan 2019 14:48:27 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <08650203BA1BD64D8AD9B6D5D74A85D160076553@SHSMSX101.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Mon, 28 Jan 2019 13:48:29 +0000 (UTC) Subject: Re: [Patch 00/33] BaseTools python3 migration patch set X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 28 Jan 2019 13:48:31 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Hi Bob, On 01/28/19 11:35, Feng, Bob C wrote: > Hi Laszlo, > > I sent out 2 patches to fix this issue. And I also pushed the patches to the repo https://github.com/BobCF/edk2.git branch py3basetools I fetched your branch, and I found two new (fast-forward-able) commits: $ git log --oneline --reverse 41c0616fc081..ec9e63692453 fb566a8835fa BaseTools: Fixed Eot python2 incompatible issue. ec9e63692453 BaseTools: Fixed Ecc python2 incompatible issue. However, the patches that you sent out: [edk2] [Patch v2 32/33] BaseTools: ECC tool Python3 adaption [edk2] [Patch v2 33/33] BaseTools: Eot tool Python3 adaption appear different; I think you squashed the above incremental fixes into the corresponding original (v1) patches. Is that correct? I prefer to test a series in the same exact state as it would be committed; otherwise there could be an issue with the rebase / squashing (possibly further tweaks), and it wouldn't be correct to apply my Tested-by to such a version. In addition, you didn't post the updated (v2) patches #32 and #33 in response to the v1 blurb: [edk2] [Patch 00/33] BaseTools python3 migration patch set and as a result they will drift apart on the list, from each other and also from the containing set. In addition, it's a good practice to keep branches unchanged once they've been posted to the list; further iterations should be named py3basetools_v2, py3basetools_v3 etc. This lets people return to earlier iterations, compare versions, associate branches with postings, and so on. Assuming the v2 32/33, v2 33/33 on-list series is what I should test, please push *precisely* that series to your repo, under branch name "py3basetools_v2". I'd also suggest resetting branch "py3basetools" to the original v1 posting status, that is, commit 41c0616fc081. Thanks Laszlo > -----Original Message----- > From: Feng, Bob C > Sent: Monday, January 28, 2019 10:33 AM > To: 'Laszlo Ersek' > Cc: edk2-devel@lists.01.org; Gao, Liming > Subject: RE: [edk2] [Patch 00/33] BaseTools python3 migration patch set > > Hi Laszlo, > > Thanks for your testing and detailed testing result. I'll check the issue happened in (a2) and provide a patch for it. > > Thanks, > Bob > > -----Original Message----- > From: Laszlo Ersek [mailto:lersek@redhat.com] > Sent: Saturday, January 26, 2019 2:18 AM > To: Feng, Bob C > Cc: edk2-devel@lists.01.org; Gao, Liming > Subject: Re: [edk2] [Patch 00/33] BaseTools python3 migration patch set > > On 01/25/19 10:42, Feng, Bob C wrote: >> [...] >> >> -----Original Message----- >> From: Laszlo Ersek [mailto:lersek@redhat.com] >> Sent: Friday, January 25, 2019 4:57 PM >> To: Feng, Bob C >> Cc: edk2-devel@lists.01.org; Gao, Liming >> Subject: Re: [edk2] [Patch 00/33] BaseTools python3 migration patch >> set >> >> [...] >> >> On 01/25/19 05:55, Feng, Bob C wrote: >>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=55 >>> >>> This patch set is to enable python3 on BaseTools. Basetools code will >>> be compatible with both python3 and python2. >>> >>> We will have two envs PYTHON3_ENABLE and PYTHON_COMMAND. The behavior >>> can be combined as the below to support this usage. >>> If user wants the specific python interpreter, he only needs to set >>> PYTHON_COMMAND env. >>> If PYTHON3_ENABLE is set, PYTHON_COMMAND will be set to the found one >>> by edk2 scripts based on PYTHON3_ENABLE value. >>> If PYTHON3_ENABLE is not set, but PYTHON_COMMAND is set, then >>> PYTHON_COMMAND will be used to run python script. No version check >>> here. >>> If PYTHON3_ENABLE is not set, but PYTHON_COMMAND is not set, >>> PYTHON_COMMAND will be set to the high version python installed in >>> OS. > > I ran the following tests, at commit 41c0616fc081 ("BaseTools: Eot tool > Python3 adaption", 2019-01-25). Each test was performed in a clean tree (after running "git clean -ffdx") and clean environment (I re-sourced "edksetup.sh" for each test in separation). In addition, the base tools were rebuilt (again from a clean tree) for each test, with the following command [1]: > > nice make -C "$EDK_TOOLS_PATH" -j $(getconf _NPROCESSORS_ONLN) > > (a) On my RHEL7.5 Workstation laptop, I have both the system-level python packages installed (python-2.7.5-69.el7_5.x86_64), and the extra > python-3.4 stuff from EPEL-7 (python34-3.4.9-1.el7.x86_64). > > (a1) Didn't set either PYTHON3_ENABLE or PYTHON_COMMAND. The build utility picked > > PYTHON_COMMAND = /usr/bin/python3.4 > > and I successfully built OvmfPkg for IA32, IA32X64, and X64; also ArmVirtQemu for AARCH64. The built firmware images passed a smoke test too. > > (a2) I removed all the python34 packages (and the dependent packages) from my laptop. Didn't set either of PYTHON3_ENABLE and PYTHON_COMMAND. > (This is the configuration what a "normal" RHEL7 environment would > provide.) > > In this case, rebuilding the base tools [1] failed, in the testing > phase: > >> ====================================================================== >> FAIL: test_Ecc_CParser4_CLexer (CheckPythonSyntax.Tests) >> ---------------------------------------------------------------------- >> Traceback (most recent call last): >> File ".../BaseTools/Tests/CheckPythonSyntax.py", line 55, in >> newmethod = lambda self: self.SingleFileTest(filename) >> File ".../BaseTools/Tests/CheckPythonSyntax.py", line 33, in SingleFileTest >> self.fail('syntax error: %s, Error is %s' % (filename, str(e))) >> AssertionError: syntax error: .../BaseTools/Source/Python/Ecc/CParser4/CLexer.py, Error is File ".../BaseTools/Source/Python/Ecc/CParser4/CLexer.py", line 590 >> def __init__(self, input=None, output:TextIO = sys.stdout): >> ^ >> SyntaxError: invalid syntax >> >> >> ====================================================================== >> FAIL: test_Ecc_CParser4_CListener (CheckPythonSyntax.Tests) >> ---------------------------------------------------------------------- >> Traceback (most recent call last): >> File ".../BaseTools/Tests/CheckPythonSyntax.py", line 55, in >> newmethod = lambda self: self.SingleFileTest(filename) >> File ".../BaseTools/Tests/CheckPythonSyntax.py", line 33, in SingleFileTest >> self.fail('syntax error: %s, Error is %s' % (filename, str(e))) >> AssertionError: syntax error: .../BaseTools/Source/Python/Ecc/CParser4/CListener.py, Error is File ".../BaseTools/Source/Python/Ecc/CParser4/CListener.py", line 35 >> def enterTranslation_unit(self, ctx:CParser.Translation_unitContext): >> ^ >> SyntaxError: invalid syntax >> >> >> ====================================================================== >> FAIL: test_Ecc_CParser4_CParser (CheckPythonSyntax.Tests) >> ---------------------------------------------------------------------- >> Traceback (most recent call last): >> File ".../BaseTools/Tests/CheckPythonSyntax.py", line 55, in >> newmethod = lambda self: self.SingleFileTest(filename) >> File ".../BaseTools/Tests/CheckPythonSyntax.py", line 33, in SingleFileTest >> self.fail('syntax error: %s, Error is %s' % (filename, str(e))) >> AssertionError: syntax error: .../BaseTools/Source/Python/Ecc/CParser4/CParser.py, Error is File ".../BaseTools/Source/Python/Ecc/CParser4/CParser.py", line 744 >> def __init__(self, input:TokenStream, output:TextIO = sys.stdout): >> ^ >> SyntaxError: invalid syntax >> >> >> ====================================================================== >> FAIL: test_Eot_CParser4_CLexer (CheckPythonSyntax.Tests) >> ---------------------------------------------------------------------- >> Traceback (most recent call last): >> File ".../BaseTools/Tests/CheckPythonSyntax.py", line 55, in >> newmethod = lambda self: self.SingleFileTest(filename) >> File ".../BaseTools/Tests/CheckPythonSyntax.py", line 33, in SingleFileTest >> self.fail('syntax error: %s, Error is %s' % (filename, str(e))) >> AssertionError: syntax error: .../BaseTools/Source/Python/Eot/CParser4/CLexer.py, Error is File ".../BaseTools/Source/Python/Eot/CParser4/CLexer.py", line 591 >> def __init__(self, input=None, output:TextIO = sys.stdout): >> ^ >> SyntaxError: invalid syntax >> >> >> ====================================================================== >> FAIL: test_Eot_CParser4_CListener (CheckPythonSyntax.Tests) >> ---------------------------------------------------------------------- >> Traceback (most recent call last): >> File ".../BaseTools/Tests/CheckPythonSyntax.py", line 55, in >> newmethod = lambda self: self.SingleFileTest(filename) >> File ".../BaseTools/Tests/CheckPythonSyntax.py", line 33, in SingleFileTest >> self.fail('syntax error: %s, Error is %s' % (filename, str(e))) >> AssertionError: syntax error: .../BaseTools/Source/Python/Eot/CParser4/CListener.py, Error is File ".../BaseTools/Source/Python/Eot/CParser4/CListener.py", line 35 >> def enterTranslation_unit(self, ctx:CParser.Translation_unitContext): >> ^ >> SyntaxError: invalid syntax >> >> >> ====================================================================== >> FAIL: test_Eot_CParser4_CParser (CheckPythonSyntax.Tests) >> ---------------------------------------------------------------------- >> Traceback (most recent call last): >> File ".../BaseTools/Tests/CheckPythonSyntax.py", line 55, in >> newmethod = lambda self: self.SingleFileTest(filename) >> File ".../BaseTools/Tests/CheckPythonSyntax.py", line 33, in SingleFileTest >> self.fail('syntax error: %s, Error is %s' % (filename, str(e))) >> AssertionError: syntax error: .../BaseTools/Source/Python/Eot/CParser4/CParser.py, Error is File ".../BaseTools/Source/Python/Eot/CParser4/CParser.py", line 744 >> def __init__(self, input:TokenStream, output:TextIO = sys.stdout): >> ^ >> SyntaxError: invalid syntax >> >> >> ---------------------------------------------------------------------- >> Ran 270 tests in 0.938s >> >> FAILED (failures=6) > > I didn't continue testing on my RHEL7 laptop beyond this point. > > > (b) RHEL-8 virtual machine, with "/usr/bin/python3.6" from python36-3.6.6-18.el8.x86_64, and "/usr/libexec/platform-python" from platform-python-3.6.8-1.el8.x86_64. > > (b1) Didn't set either PYTHON3_ENABLE or PYTHON_COMMAND. The build utility picked > > PYTHON_COMMAND = /usr/bin/python3.6 > > and I successfully built OvmfPkg for IA32, IA32X64, and X64. (I don't have a cross-compiler installed in this environment yet, nor a RHEL8 > aarch64 KVM guest, so I couldn't test ArmVirtQemu for now). > > (b2) I set PYTHON_COMMAND to "/usr/libexec/platform-python". Didn't set PYTHON3_ENABLE. The same builds as in (b1) succeeded. > > > Thus, what seems to be missing is a little bit of python2 compatibility, in (a2). > > Thanks, > Laszlo > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel >