Re: [PATCH v2] livepatch-build-tools: allow livepatching version.c

From: "Roger Pau Monné" <roger.pau@citrix.com>
To: Ross Lagerwall <ross.lagerwall@cloud.com>
Cc: xen-devel@lists.xenproject.org,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [PATCH v2] livepatch-build-tools: allow livepatching version.c
Date: Fri, 15 Dec 2023 14:53:26 +0100	[thread overview]
Message-ID: <ZXxaVq19etLNbjEl@macbook> (raw)
In-Reply-To: <CAG7k0EqHi2gTFY8rhuceVURTWv1TvHLuQgkvjtjoz=A7GWdiAQ@mail.gmail.com>

On Fri, Dec 15, 2023 at 11:47:18AM +0000, Ross Lagerwall wrote:
> On Thu, Dec 14, 2023 at 8:54 AM Roger Pau Monne <roger.pau@citrix.com> wrote:
> >
> > Currently version.o is explicitly ignored as the file would change as a result
> > of the orignal and the patched build having possibly different dates, times or
> > changeset strings (by the patched build appending -dirty).
> >
> > Fix such difference by exporting the date and time from the build script, so
> > that both builds share the same build time.  The changeset string gets set in
> > .scmversion, and is cleaned on script exit if it wasn't present initially. This
> > allows checking for changes in version.c, since the rest of fields need to be
> > manually changed in order to produce different output.
> >
> > Setting XEN_BUILD_{DATE,TIME} as an environment variable and .scmversion has
> > been supported since before livepatch support was added to Xen, so it's safe to
> > export those variables unconditionally.
> >
> > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > ---
> > Changes since v1:
> >  - Also ensure consistent changeset version.
> > ---
> >  livepatch-build | 14 ++++++++++++++
> >  livepatch-gcc   |  2 --
> >  2 files changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/livepatch-build b/livepatch-build
> > index e2ccce4f7fd7..332d26f5c6b6 100755
> > --- a/livepatch-build
> > +++ b/livepatch-build
> > @@ -392,6 +392,10 @@ echo "Output directory: $outputarg"
> >  echo "================================================"
> >  echo
> >
> > +cleanup_version() {
> > +    rm -rf "${SRCDIR}/xen/.scmversion"
> > +}
> > +
> >  if [ "${SKIP}" != "build" ]; then
> >      # Make sure output directory doesn't exist, and create it.
> >      [ -e "$outputarg" ] && die "Output directory exists"
> > @@ -417,6 +421,16 @@ if [ "${SKIP}" != "build" ]; then
> >
> >      export CROSS_COMPILE="${TOOLSDIR}/livepatch-gcc "
> >
> > +    # Force same date and time to prevent unwanted changes in version.c
> > +    export XEN_BUILD_DATE=`LC_ALL=C date`
> > +    export XEN_BUILD_TIME=`LC_ALL=C date +%T`
> > +
> > +    # Ensure uniform changeset between builds
> > +    if [ ! -e "${SRCDIR}/xen/.scmversion" ]; then
> > +        trap cleanup_version EXIT
> > +        echo "unavailable" > "${SRCDIR}/xen/.scmversion"
> 
> Shouldn't this be "${SRCDIR}/.scmversion" since Xen runs
> "$(srctree)/tools/scmversion $(XEN_ROOT)"?
> 
> Otherwise I still get spurious version.o changes...
> 
> With the 3 uses of that changed (can change during commit),
> 
> Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>

Great, I had a stale .scmversion in my Xen root and that's why I
wasn't seeing the path not being effective.

Please adjust on commit.

Thanks, Roger.