From: "Mcnamara, John" <john.mcnamara@intel.com>
To: Neil Horman <nhorman@tuxdriver.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
"mkletzan@redhat.com" <mkletzan@redhat.com>
Subject: Re: [dpdk-dev] [PATCH v1 0/4] app: make python apps python2/3 compliant
Date: Fri, 9 Dec 2016 17:00:19 +0000 [thread overview]
Message-ID: <B27915DBBA3421428155699D51E4CFE202671EE0@IRSMSX103.ger.corp.intel.com> (raw)
In-Reply-To: <20161209152836.GA23061@hmsreliant.think-freely.org>
> -----Original Message-----
> From: Neil Horman [mailto:nhorman@tuxdriver.com]
> Sent: Friday, December 9, 2016 3:29 PM
> To: Mcnamara, John <john.mcnamara@intel.com>
> Cc: dev@dpdk.org; mkletzan@redhat.com
> Subject: Re: [dpdk-dev] [PATCH v1 0/4] app: make python apps python2/3
> compliant
>
> On Thu, Dec 08, 2016 at 03:51:01PM +0000, John McNamara wrote:
> > These patches refactor the DPDK Python applications to make them
> > Python 2/3 compatible.
> >
> > In order to do this the patchset starts by making the apps PEP8
> > compliant in accordance with the DPDK Coding guidelines:
> >
> >
> > http://dpdk.org/doc/guides/contributing/coding_style.html#python-code
> >
> > Implementing PEP8 and Python 2/3 compliance means that we can check
> > all future Python patches for consistency. Python 2/3 support also
> > makes downstream packaging easier as more distros move to Python 3 as
> the system python.
> >
> > See the previous discussion about Python2/3 compatibilty here:
> >
> > http://dpdk.org/ml/archives/dev/2016-December/051683.html
> >
> > I've tested that the apps compile with python 2 and 3 and I've tested
> > some of the apps for consistent output but it needs additional testing.
> >
> > John McNamara (4):
> > app: make python apps pep8 compliant
> > app: make python apps python2/3 compliant
> > app: give python apps a consistent shebang line
> > doc: add required python versions to coding guidelines
> >
> > app/cmdline_test/cmdline_test.py | 87 ++-
> > app/cmdline_test/cmdline_test_data.py | 403 +++++-----
> > app/test/autotest.py | 46 +-
> > app/test/autotest_data.py | 831 +++++++++++---
> -------
> > app/test/autotest_runner.py | 740 +++++++++-----
> ----
> > app/test/autotest_test_funcs.py | 481 ++++++------
> > doc/guides/conf.py | 11 +-
> > doc/guides/contributing/coding_style.rst | 3 +-
> > examples/ip_pipeline/config/diagram-generator.py | 13 +-
> > .../ip_pipeline/config/pipeline-to-core-mapping.py | 11 +-
> > tools/cpu_layout.py | 79 +-
> > tools/dpdk-devbind.py | 26 +-
> > tools/dpdk-pmdinfo.py | 73 +-
> > 13 files changed, 1410 insertions(+), 1394 deletions(-)
> >
> > --
> > 2.7.4
> >
> I think the changelog is deceptive. It claims to make all the utilities
> python2 and 3 compliant. But compliance with python3 is more than just
> stylistic formatting. After this series several of these apps continue to
> fail under python3. dpdk-pmdinfo as an example:
>
> [nhorman@hmsreliant dpdk]$ ./tools/dpdk-pmdinfo.py ./build/app/testacl
> Traceback (most recent call last):
> File "./tools/dpdk-pmdinfo.py", line 607, in <module>
> main()
> File "./tools/dpdk-pmdinfo.py", line 596, in main
> readelf.process_dt_needed_entries()
> File "./tools/dpdk-pmdinfo.py", line 437, in process_dt_needed_entries
> rc = tag.needed.find("librte_pmd")
> TypeError: a bytes-like object is required, not 'str'
>
>
> I'm not saying its a bad patchset, but the changelog should reflect that
> the change is purely stylistic, not functional.
>
Hi Neil,
Mea cupla. In my defense I did say in the cover letter that I'd tested that the apps compiled but that they needed extra testing. I did functionally test some of the apps that I was more familiar with, but not all of them. In particular the test apps need functional testing.
However, the changes need to be functional rather than just cosmetic so I'll look into fixing pmdinfo with Python 3, unless you'd prefer to do that ;-). Since pmdinfo is dealing with binary data it may be tricky. That is often one of the real challenges of porting Python 2 code to Python 3. Hopefully elftools is compatible. Anyway I'll look into it.
And just to be clear, I don't think this patchset should be merged until all of the apps have been functionally tested. I'll put something in the final patchset to indicate that the modified apps have been tested.
John
next prev parent reply other threads:[~2016-12-09 17:00 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-08 15:51 John McNamara
2016-12-08 15:51 ` [dpdk-dev] [PATCH v1 1/4] app: make python apps pep8 compliant John McNamara
2016-12-08 15:51 ` [dpdk-dev] [PATCH v1 2/4] app: make python apps python2/3 compliant John McNamara
2016-12-08 15:51 ` [dpdk-dev] [PATCH v1 3/4] app: give python apps a consistent shebang line John McNamara
2016-12-08 15:51 ` [dpdk-dev] [PATCH v1 4/4] doc: add required python versions to coding guidelines John McNamara
2016-12-08 16:03 ` [dpdk-dev] [PATCH v2 1/4] app: make python apps pep8 compliant John McNamara
2016-12-08 16:03 ` [dpdk-dev] [PATCH v2 2/4] app: make python apps python2/3 compliant John McNamara
2016-12-08 16:03 ` [dpdk-dev] [PATCH v2 3/4] app: give python apps a consistent shebang line John McNamara
2016-12-08 16:20 ` Thomas Monjalon
2016-12-08 20:44 ` Mcnamara, John
2016-12-08 16:03 ` [dpdk-dev] [PATCH v2 4/4] doc: add required python versions to coding guidelines John McNamara
2016-12-09 15:28 ` [dpdk-dev] [PATCH v1 0/4] app: make python apps python2/3 compliant Neil Horman
2016-12-09 17:00 ` Mcnamara, John [this message]
2016-12-09 17:06 ` Neil Horman
2016-12-09 17:41 ` Mcnamara, John
2016-12-18 14:25 ` [dpdk-dev] [PATCH v2 0/3] " John McNamara
2016-12-18 14:25 ` [dpdk-dev] [PATCH v2 1/3] app: make python apps pep8 compliant John McNamara
2016-12-18 14:25 ` [dpdk-dev] [PATCH v2 2/3] app: make python apps python2/3 compliant John McNamara
2016-12-18 14:25 ` [dpdk-dev] [PATCH v2 3/3] doc: add required python versions to docs John McNamara
2016-12-18 14:32 ` [dpdk-dev] [PATCH v3 0/3] app: make python apps python2/3 compliant John McNamara
2016-12-18 14:32 ` [dpdk-dev] [PATCH v3 1/3] app: make python apps pep8 compliant John McNamara
2016-12-18 14:32 ` [dpdk-dev] [PATCH v3 2/3] app: make python apps python2/3 compliant John McNamara
2016-12-18 14:32 ` [dpdk-dev] [PATCH v3 3/3] doc: add required python versions to docs John McNamara
2016-12-21 15:03 ` [dpdk-dev] [PATCH v4 0/3] app: make python apps python2/3 compliant John McNamara
2017-01-04 20:15 ` Thomas Monjalon
2016-12-21 15:03 ` [dpdk-dev] [PATCH v4 1/3] app: make python apps pep8 compliant John McNamara
2016-12-21 15:03 ` [dpdk-dev] [PATCH v4 2/3] app: make python apps python2/3 compliant John McNamara
2016-12-21 15:03 ` [dpdk-dev] [PATCH v4 3/3] doc: add required python versions to docs John McNamara
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-all 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=B27915DBBA3421428155699D51E4CFE202671EE0@IRSMSX103.ger.corp.intel.com \
--to=john.mcnamara@intel.com \
--cc=dev@dpdk.org \
--cc=mkletzan@redhat.com \
--cc=nhorman@tuxdriver.com \
/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;
as well as URLs for NNTP newsgroup(s).