Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <Y49vx0v6Z7EiR8jr@kernel.org> <Y49wxSIK7dJ7iTDg@kernel.org>
 <Y491d1wEW4TfUi5f@kernel.org> <Y4921D+36UGdhK92@kernel.org>
 <Y494TNa0ZyPH9YSD@kernel.org> <Y498YP2N3gvFSr/X@kernel.org>
 <CAP-5=fW2Fdfo9njgXxCVDP0dF3gTsUtaPMh88uSC5bRVjp+1Uw@mail.gmail.com>
 <Y5ChXjt0uv/yDNwV@kernel.org> <Y5Cjylv9dJh796dw@kernel.org>
 <20221207093958.09ae35c2@gandalf.local.home> <Y5C5AZ1YfthY0tx2@kernel.org>
In-Reply-To: <Y5C5AZ1YfthY0tx2@kernel.org>
From:   Ian Rogers <irogers@google.com>
Date:   Wed, 7 Dec 2022 09:00:27 -0800
Message-ID: <CAP-5=fWBzNP=uDLdyODC8gvT3KT5Kz9rRNouyLn=22_ouD8wEg@mail.gmail.com>
Subject: Re: [ALMOST ready] Re: [PATCH 2/3] perf build: Use libtraceevent from
 the system
To:     Arnaldo Carvalho de Melo <acme@kernel.org>
Cc:     Steven Rostedt <rostedt@goodmis.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Jiri Olsa <jolsa@kernel.org>,
        Namhyung Kim <namhyung@kernel.org>,
        Nick Desaulniers <ndesaulniers@google.com>,
        linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
        bpf@vger.kernel.org, Stephane Eranian <eranian@google.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Dec 7, 2022 at 8:02 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> Em Wed, Dec 07, 2022 at 09:39:58AM -0500, Steven Rostedt escreveu:
> > On Wed, 7 Dec 2022 11:31:38 -0300
> > Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> >
> > > No distro I tested so far has a package for libtracevent in is default
> > > repositories:
> >
> > Not sure what you mean by "default repository".
> >
> > At least on Debian testing, I have libtraceevent-dev available.
>
> Right, I'm talking about non-bleeding edge, distros that are still
> supported. I'm still checking, fedora is ok all the way back to 33, wrt
> having libtraceevent available.
>
> - Arnaldo

I think this is both similar to the python2 issue and different. With
python we now have a build time dependency on having at least Python
3.6 if you want json events. With this change you need the system to
have libtraceevent if you want to have various commands like perf
sched, perf trace, etc. Losing entire commands with this change is
more than happened with the Python 3.6 dependency. With the python
issue, you can fix the build issue by installing Python 3.6 and with
this issue you can fix it by installing libtraceevent-devel. In both
cases you can install via a package manager, but given you are likely
building the perf tool from source, you can also build both python and
libtraceevent from source. Saying that an old distro is broken by this
change, or the Python 3.6 change, I think is too strong and we're in
danger of holding things back for a very small (possibly 0) set of
people. They'd have to be upset about building from source on an old
distro, then losing the less commonly used commands of perf sched,
perf trace, etc. and be unwilling to build libtraceevent from source.
Given they are building the perf tool from source I just can't imagine
they'd be too upset about building libtraceevent from source.

I think a problem with both changes is that the build time warning
given isn't loud enough. Perhaps we can make it that at build time we
$(error for these recommended dependencies, unless NO_JEVENTS=1 or
NO_LIBTRACEEVENT=1 are added. Losing functionality and just having a
small $(warning seems error prone and likely to cause functionality to
be accidentally lost.

Thanks,
Ian