Re: [PATCH] perf scripts python: Let script to be python2 compliant

[Chuck Zmudzinski <brchuckz@netscape.net>]

On 8/17/22 6:36 PM, Ian Rogers wrote:
> On Wed, Aug 17, 2022 at 3:13 PM Chuck Zmudzinski <brchuckz@netscape.net> wrote:
> >
> > On 8/17/2022 3:52 PM, Chuck Zmudzinski wrote:
> > > On 7/26/22 4:43 PM, Ian Rogers wrote:
> > > > On Tue, Jul 26, 2022 at 12:43 PM Arnaldo Carvalho de Melo
> > > > <acme@kernel.org> wrote:
> > > > >
> > > > > Em Tue, Jul 26, 2022 at 10:52:31AM -0700, Ian Rogers escreveu:
> > > > > > On Tue, Jul 26, 2022 at 9:57 AM Alan Bartlett <ajb@elrepo.org> wrote:
> > > > > > >
> > > > > > > On Mon, 25 Jul 2022 at 16:51, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > > > > > >
> > > > > > > > Em Mon, Jul 25, 2022 at 06:42:20PM +0800, Leo Yan escreveu:
> > > > > > > > > The mainline kernel can be used for relative old distros, e.g. RHEL 7.
> > > > > > > > > The distro doesn't upgrade from python2 to python3, this causes the
> > > > > > > > > building error that the python script is not python2 compliant.
> > > > > > > > >
> > > > > > > > > To fix the building failure, this patch changes from the python f-string
> > > > > > > > > format to traditional string format.
> > > > > > > >
> > > > > > > > Thanks, applied.
> > > > > > > >
> > > > > > > > - Arnaldo
> > > > > > >
> > > > > > > Leo / Arnaldo,
> > > > > > >
> > > > > > > Applying the patch on top of -5.19-rc8 fixes the problem that we (the
> > > > > > > ELRepo Project) experienced when attempting to build on RHEL7.
> > > > > > >
> > > > > > > So --
> > > > > > >
> > > > > > > Tested-by: Alan Bartlett <ajb@elrepo.org>
> > > > > > >
> > > > > > > Hopefully you will get it to Linus in time for -5.19 GA.
> > > > >
> > > > > > So I'm somewhat concerned about perf supporting unsupported
> > > > > > distributions and this holding the code base back. RHEL7 was launched
> > > > > > 8 years ago (June 10, 2014) and full support ended 3 years ago (August
> > > > > > 6, 2019) [1]. Currently RHEL7 is in "Maintenance Support or
> > > > > > Maintenance Support 2" phase which is defined to mean [2]:
> > > > > >
> > > > > > ```
> > > > > > During the Maintenance Support Phase for Red Hat Enterprise Linux
> > > > > > Version 8 & 9, and Maintenance Support 2 Phase for Red Hat Enterprise
> > > > > > Linux version 7, Red Hat defined Critical and Important impact
> > > > > > Security Advisories (RHSAs) and selected (at Red Hat discretion)
> > > > > > Urgent Priority Bug Fix Advisories (RHBAs) may be released as they
> > > > > > become available. Other errata advisories may be delivered as
> > > > > > appropriate.
> > > > > >
> > > > > > New functionality and new hardware enablement are not planned for
> > > > > > availability in the Maintenance Support (RHEL 8 & 9) Phase and
> > > > > > Maintenance Support 2 (RHEL 7) Phase.
> > > > > > ```
> > > > > >
> > > > > > >From this definition, why would RHEL7 pick up a new perf tool? I don't
> > > > > > think they would and as such we don't need to worry about supporting
> > > > > > it. RHEL8 defaults to python 3 and full support ends for it next year.
> > > > > > Let's set the bar at RHEL8 and not worry about RHEL7 breakages like
> > > > > > this in future. I think the bar for caring should be "will the distro
> > > > > > pick up our code", if we don't do this then we're signing up to not
> > > > > > allowing tools to update for 10 years! If someone is building a kernel
> > > > > > and perf tool on RHEL7 then they should be signing up to also deal
> > > > > > with tool chain issues, which in this case can mean installing
> > > > > > python3.
> > > > >
> > > > > In this specific supporting things that people report using, like was
> > > > > done in this case, isn't such a big problem.
> > > >
> > > > So there are linters will fire for this code and say it is not
> > > > pythonic. It is only a linter warning vs asking to support an 8 year
> > > > old out of support distribution. There are other cases, such as
> > > > improving the C code structure, where we've failed to land changes
> > > > because of build errors on old distributions. This could indicate perf
> > > > code is wrong or the distribution is wrong. I'm saying that if we
> > > > believe in the perf code being correct and the distribution is out of
> > > > support, then we should keep the perf code as-is and the issue is one
> > > > for user of the out-of-support distribution.
> > > >
> > > > > Someone reported a problem in a system they used, the author of the code
> > > > > in question posted a patch allowing perf to be used in such old systems,
> > > > > doesn't get in the way of newer systems, small patch, merged, life goes
> > > > > on.
> > >
> > > Considering the proposed patch, can you be sure that replacing the
> > > f-string format with the legacy format won't cause a regression for
> > > some python3 user somewhere when this hits the real world? Even
> > > if it does not cause a regression today, as new versions and features
> > > are added to python3, can you be sure none of those new features
> > > will depend on the upgrade from the legacy format to the f-string
> > > format here to work properly? So many regressions happen because
> > > the people who write patches cannot possibly foresee how their
> > > patch is going to affect the millions of Linux users out there, but still
> > > they are certain it will not cause a regression somewhere. So how
> > > can the chances that this patch will cause a regression be minimized?
> > >
> > > It seems to me for this to be suitable for the Linux kernel, the
> > > default should be to use the modern python3 format and only
> > > enable python2 compatibility via a sysctl setting and/or kernel boot
> > > option for those who are still using python2. There should be no
> > > change to the behavior of the kernel for users who have upgraded
> > > to python3. But I don't see any such consideration for python3
> > > users in this patch.
> >
> > Sorry, I didn't see this is a script, LOL! So obviously a sysctl or boot option
> > does not apply. But can't the script implement this simple logic:
> >
> > If python version = 3 use f-string format
> > if python version = 2 use traditional string format
>
> Doing this in the script would be noisy, having two scripts less than
> ideal. I'd suggest we wait two weeks, declare the official death of
> RHEL7 without "rpm -i python3" and then revert the python3 to python2
> patch. There are plenty of things to worry about and python2 shouldn't
> be one of them (it died over 2 years ago).

I see this has already been committed. I agree it should not
stay in the kernel tree for long. At some point in the future
it will most likely cause problems if it is not reverted.

Chuck

>
> Thanks,
> Ian
>
> > Best regards,
> >
> > Chuck