Message-ID: <CALsPMBNSJQXz4zG_s=f1E6UbY-m98ibtnHoUDBPLq+x9wMQ5cg@mail.gmail.com>
Date: Sun, 16 Jun 2024 05:19:44 +0200
From: Roman Storozhenko <romeusmeister@...il.com>
To: Shuah Khan <skhan@...uxfoundation.org>
Cc: Thomas Renninger <trenn@...e.com>, Shuah Khan <shuah@...nel.org>, 
	Javier Carrasco <javier.carrasco.cruz@...il.com>, linux-pm@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] cpupower: Improve cpupower build process description

On Sun, Jun 16, 2024 at 1:05 AM Shuah Khan <skhan@...uxfoundation.org> wrote:
>
> On 6/15/24 06:56, Roman Storozhenko wrote:
> > Enhance cpupower build process description with the information on
> > building and installing the utility to the user defined directories
> > as well as with the information on the way of running the utility from
> > the custom defined installation directory.
> >
> > Signed-off-by: Roman Storozhenko <romeusmeister@...il.com>
> > ---
> > V1 -> V2:
> >   - Improved commit description
> >   - Make changed line lenghts 75 chars
> >   - Refactored the description
> >   - Link v1: https://lore.kernel.org/linux-pm/20240613-fix-cpupower-doc-v1-1-9dcdee263af1@gmail.com/
> > ---
> >   tools/power/cpupower/README | 160 +++++++++++++++++++++++++++++++++---
> >   1 file changed, 150 insertions(+), 10 deletions(-)
> >
> > diff --git a/tools/power/cpupower/README b/tools/power/cpupower/README
> > index 1c68f47663b2..2678ed81d311 100644
> > --- a/tools/power/cpupower/README
> > +++ b/tools/power/cpupower/README
> > @@ -22,16 +22,156 @@ interfaces [depending on configuration, see below].
> >   compilation and installation
> >   ----------------------------
> >
> > -make
> > -su
> > -make install
> > -
> > -should suffice on most systems. It builds libcpupower to put in
> > -/usr/lib; cpupower, cpufreq-bench_plot.sh to put in /usr/bin; and
> > -cpufreq-bench to put in /usr/sbin. If you want to set up the paths
> > -differently and/or want to configure the package to your specific
> > -needs, you need to open "Makefile" with an editor of your choice and
> > -edit the block marked CONFIGURATION.
> > +There are 2 output directories - one for the build output and another for
> > +the installation of the build results, that is the utility, library,
> > +man pages, etc...
> > +
> > +default directory
> > +-----------------
> > +
> > +In the case of default directory, build and install process requires no
> > +additional parameters:
> > +
> > +build
> > +-----
> > +
> > +$ make
> > +
> > +The output directory for the 'make' command is the current directory and
> > +its subdirs in the kernel tree:
> > +tools/power/cpupower
> > +
> > +install
> > +-------
> > +
> > +$ sudo make install
> > +
> > +'make install' command puts targets to default system dirs:
> > +
> > +-----------------------------------------------------------------------
> > +| Installing file        |               System dir                   |
> > +-----------------------------------------------------------------------
> > +| libcpupower            | /usr/lib                                   |
> > +-----------------------------------------------------------------------
> > +| cpupower               | /usr/bin                                   |
> > +-----------------------------------------------------------------------
> > +| cpufreq-bench_plot.sh  | /usr/bin                                   |
> > +-----------------------------------------------------------------------
> > +| man pages              | /usr/man                                   |
> > +-----------------------------------------------------------------------
> > +
> > +To put it in other words it makes build results available system-wide,
> > +enabling any user to simply start using it without any additional steps
> > +
> > +custom directory
> > +----------------
> > +
> > +There are 2 make's command-line variables 'O' and 'DESTDIR' that setup
> > +appropriate dirs:
> > +'O' - build directory
> > +'DESTDIR' - installation directory. This variable could also be setup in
> > +the 'CONFIGURATION' block of the "Makefile"
> > +
> > +build
> > +-----
> > +
> > +$ make O=<your_custom_build_catalog>
> > +
> > +Example:
> > +$ make O=/home/hedin/prj/cpupower/build
> > +
> > +install
> > +-------
> > +
> > +$ make O=<your_custom_build_catalog> DESTDIR=<your_custom_install_catalog>
> > +
> > +Example:
> > +$ make O=/home/hedin/prj/cpupower/build DESTDIR=/home/hedin/prj/cpupower \
> > +> install
> > +
> > +Notice that both variables 'O' and 'DESTDIR' have been provided. The reason
> > +is that the build results are saved in the custom output dir defined by 'O'
> > +variable. So, this dir is the source for the installation step. If only
> > +'DESTDIR' were provided then the 'install' target would assume that the
> > +build directory is the current one, build everything there and install
> > +from the current dir.
> > +
> > +The files will be installed to the following dirs:
> > +
> > +-----------------------------------------------------------------------
> > +| Installing file        |               System dir                   |
> > +-----------------------------------------------------------------------
> > +| libcpupower            | ${DESTDIR}/usr/lib                         |
> > +-----------------------------------------------------------------------
> > +| cpupower               | ${DESTDIR}/usr/bin                         |
> > +-----------------------------------------------------------------------
> > +| cpufreq-bench_plot.sh  | ${DESTDIR}/usr/bin                         |
> > +-----------------------------------------------------------------------
> > +| man pages              | ${DESTDIR}/usr/man                         |
> > +-----------------------------------------------------------------------
> > +
> > +If you look at the table for the default 'make' output dirs you will
> > +notice that the only difference with the non-default case is the
> > +${DESTDIR} prefix. So, the structure of the output dirs remains the same
> > +regardles of the root output directory.
> > +
> > +
> > +clean and uninstall
> > +-------------------
> > +
> > +'clean' target is intended for cleanup the build catalog from build results
> > +'uninstall' target is intended for removing installed files from the
> > +installation directory
> > +
> > +default directory
> > +-----------------
> > +
> > +This case is a straightforward one:
> > +$ make clean
> > +$ make uninstall
> > +
> > +custom directory
> > +----------------
> > +
> > +Use 'O' command line variable to remove previously built files from the
> > +build dir:
> > +$ make O=<your_custom_build_catalog> clean
> > +
> > +Example:
> > +$ make O=/home/hedin/prj/cpupower/build clean
> > +
> > +Use 'DESTDIR' command line variable to uninstall previously installed files
> > +from the given dir:
> > +$ make DESTDIR=<your_custom_install_catalog>
> > +
> > +Example:
> > +make DESTDIR=/home/hedin/prj/cpupower uninstall
> > +
> > +
> > +running the tool
> > +----------------
> > +
> > +default directory
> > +-----------------
> > +
> > +$ sudo cpupower
> > +
> > +custom directory
> > +----------------
> > +
> > +When it comes to run the utility from the custom build catalog things
> > +become a little bit complicated as 'just run' approach doesn't work.
> > +Assuming that the current dir is '<your_custom_install_catalog>/usr',
> > +issuing the following command:
> > +
> > +$ sudo ./bin/cpupower
> > +will produce the following error output:
> > +./bin/cpupower: error while loading shared libraries: libcpupower.so.1:
> > +cannot open shared object file: No such file or directory
> > +
> > +The issue is that binary cannot find the 'libcpupower' library. So, we
> > +shall point to the lib dir:
> > +sudo LD_LIBRARY_PATH=lib64/ ./bin/cpupower
> >
> >
> >   THANKS
>
> This "THANKS" doesn't belong in the patch.

The 'THANKS' does belong to the patch as well as the 'interfaces
[depending on configuration, see below]' line at the top.
Those 2 lines are parts of the original file and show the bottom and
the top of  the changed text area.
Just in case, I tried to re-add my changes using 'git add -i' on my
dev machine and found that it is impossible to get rid of it.
And it's explainable, git wants to know the area of the text to change.
Besides, applying the downloaded patch using "git am' doesn't
introduce the second 'THANKS' word at the bottom of the text.
That is, the patch applies correctly.
I decided to experiment further and removed the 'THANKS' line from the
patch that had been sent and checked it with 'checkpatch':
hedin@...top:~/lkmp/patchwork/patches/powertools/cpupower/update_doc_install/tmp$
~/prj/linux/scripts/checkpatch.pl
0001-cpupower-Improve-cpupower-build-process-description.patch
                                                       ERROR: patch
seems to be corrupt (line wrapped?)
#193: FILE: tools/power/cpupower/README:176:

Returned the 'THANKS':
hedin@...top:~/lkmp/patchwork/patches/powertools/cpupower/update_doc_install/tmp$
~/prj/linux/scripts/checkpatch.pl
0001-cpupower-Improve-cpupower-build-process-description.patch
total: 0 errors, 0 warnings, 166 lines checked
0001-cpupower-Improve-cpupower-build-process-description.patch has no
obvious style problems and is ready for submission.

So, as you can see 'THANKS' is a required part of the patch.
>
> thanks,
> -- Shuah
>


-- 
Kind regards,
Roman Storozhenko

Powered by blists - more mailing lists