Re: [dpdk-dev] [PATCH v1 0/4] app: make python apps python2/3 compliant

["Mcnamara, John" <john.mcnamara@intel.com>]

> -----Original Message-----
> From: Neil Horman [mailto:nhorman@tuxdriver.com]
> Sent: Friday, December 9, 2016 5:06 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 Fri, Dec 09, 2016 at 05:00:19PM +0000, Mcnamara, John wrote:
> >
> >
> > > -----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-c
> > > > ode
> > > >
> > > > 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
> > > >
> > > ...
> >
> > 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.
> >
> I completely agree that the utilities should be functionally compatible
> with python 3, but in regards to this patch set, all I'm really asking for
> is that the changelog reflect that its just making stylistic changes to
> comply with pep8 (i.e. not fixing python 2/3 compatibility issues).  The
> latter can be handled in subsequent patches piecemeal.
> 

Hi Neil,

Sorry if I'm missing something, but isn't that how the changelog is structured?

>From above:

 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

The PEP8 changes are made in 1/4. The Python 2/3 changes are made in 2/4.

Also, the pmdinfo issue was easier to resolve than I feared. The following should fix the exception (on top of the other patches). I tested the output with a couple of pmds but maybe
you could verify it as well.

John

$ git diff
diff --git a/tools/dpdk-pmdinfo.py b/tools/dpdk-pmdinfo.py
index 097982e..d4e51aa 100755
--- a/tools/dpdk-pmdinfo.py
+++ b/tools/dpdk-pmdinfo.py
@@ -434,7 +434,7 @@ def process_dt_needed_entries(self):
 
         for tag in dynsec.iter_tags():
             if tag.entry.d_tag == 'DT_NEEDED':
-                rc = tag.needed.find("librte_pmd")
+                rc = tag.needed.find(b"librte_pmd")
                 if (rc != -1):
                     library = search_file(tag.needed,