Message-ID: <CAP-5=fXjBBbU3EHD5iY6VMKMo3wj+dJteeqBf5B9m464HtdF3Q@mail.gmail.com>
Date:   Wed, 17 Aug 2022 15:36:49 -0700
From:   Ian Rogers <irogers@...gle.com>
To:     Chuck Zmudzinski <brchuckz@...scape.net>
Cc:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        Alan Bartlett <ajb@...epo.org>, Leo Yan <leo.yan@...aro.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        linux-perf-users@...r.kernel.org, ElRepo <contact@...epo.org>,
        Akemi Yagi <toracat@...epo.org>, Jiri Olsa <jolsa@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Namhyung Kim <namhyung@...nel.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] perf scripts python: Let script to be python2 compliant

On Wed, Aug 17, 2022 at 3:13 PM Chuck Zmudzinski <brchuckz@...scape.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@...nel.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@...epo.org> wrote:
> > > > > >
> > > > > > On Mon, 25 Jul 2022 at 16:51, Arnaldo Carvalho de Melo <acme@...nel.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@...epo.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).

Thanks,
Ian

> Best regards,
>
> Chuck

Powered by blists - more mailing lists