[PATCH] buildversion.h: use the last git commit as timesource if tree is clean

[Alexander Couzens]

Additional set TZ=UTC to have predictable `date` output.

Signed-off-by: Alexander Couzens 
---
 scripts/buildversion.sh | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/scripts/buildversion.sh b/scripts/buildversion.sh
index e5ce96c..8388afd 100755
--- a/scripts/buildversion.sh
+++ b/scripts/buildversion.sh
@@ -2,16 +2,22 @@
 # Script to generate a C file with version information.
 OUTFILE="$1"
 VAR16MODE="$2"
+DATE=$(date +%s)

# Extract version info
 if [ -d .git -o -f .git ]; then
     VERSION="`git describe --tags --long --dirty`"
+    echo "$VERSION" | grep -q -- "-dirty$"
+    if [ $? -eq 1 ]; then
+        # clean tree use git date
+        DATE="$(git log --date=local --pretty=format:%ct -1)"
+    fi
 elif [ -f .version ]; then
     VERSION="`cat .version`"
 else
     VERSION="?"
 fi
-VERSION="${VERSION}-`date +"%Y%m%d_%H%M%S"`-`hostname`"
+VERSION="${VERSION}-$(TZ=UTC date -d @$DATE +%Y%m%d_%H%M%S)-`hostname`"
 echo "Version: ${VERSION}"

# Build header file

[PATCH] buildversion.h: use the last git commit as timesource if tree is clean

[Alexander Couzens]

Sorry for the noise, I missed the last patch from Gerd Hoffmann
build: add opt-out for timestamp and hostname.

Please ignore mine if we are merging that one.

best
lynxis

ps: `date -d` isn't portable it needs an v2 anyway. coreboot is using uname 
to define the right argument switch.

[PATCH] buildversion.h: use the last git commit as timesource if tree is clean

[Kevin O'Connor]

On Thu, Jun 04, 2015 at 01:25:52AM +0200, Alexander Couzens wrote:
> Sorry for the noise, I missed the last patch from Gerd Hoffmann
> build: add opt-out for timestamp and hostname.
>
> Please ignore mine if we are merging that one.

Gerd, it doesn't look like you pushed your "add opt-out" patch.  Are
you okay with the patch below instead?  It supports a "make
VERSION=xyz" build option that gives the user explicit control of the
contents of the version string.

-Kevin

commit 624e812764beda88b47c0018b1cee3b86d5c59eb
Author: Kevin O'Connor 
Date:   Wed Jun 3 19:17:12 2015 -0400

build: Support "make VERSION=xyz" to override the default build version

Add a build option to explicitly set the version information compiled
    into the seabios and seavgabios binaries.  This may assist in
    reproducible builds or to better link builds to distribution packages.
    If the new "VERSION=" parameter is not provided then the default build
    version remains unchanged.

Signed-off-by: Kevin O'Connor

diff --git a/Makefile b/Makefile
index 96aec48..f97b1bd 100644
--- a/Makefile
+++ b/Makefile
@@ -155,7 +155,7 @@ $(OUT)romlayout.o: src/romlayout.S $(OUT)autoconf.h $(OUT)asm-offsets.h

$(OUT)romlayout16.lds: $(OUT)ccode32flat.o $(OUT)code32seg.o $(OUT)ccode16.o $(OUT)romlayout.o scripts/layoutrom.py scripts/buildversion.sh
        @echo "  Building ld scripts"
-       $(Q)./scripts/buildversion.sh $(OUT)version.c
+       $(Q)BUILD_VERSION="$(VERSION)" ./scripts/buildversion.sh $(OUT)version.c
        $(Q)$(CC) $(CFLAGS32FLAT) -c $(OUT)version.c -o $(OUT)version.o
        $(Q)$(LD) $(LD32BIT_FLAG) -r $(OUT)ccode32flat.o $(OUT)version.o -o $(OUT)code32flat.o
        $(Q)$(LD) $(LD32BIT_FLAG) -r $(OUT)ccode16.o $(OUT)romlayout.o -o $(OUT)code16.o
@@ -227,7 +227,7 @@ $(OUT)vgaentry.o: vgasrc/vgaentry.S $(OUT)autoconf.h $(OUT)asm-offsets.h

$(OUT)vgarom.o: $(OUT)vgaccode16.o $(OUT)vgaentry.o $(OUT)vgasrc/vgalayout.lds scripts/buildversion.sh
        @echo "  Linking $@"
-       $(Q)./scripts/buildversion.sh $(OUT)vgaversion.c VAR16
+       $(Q)BUILD_VERSION="$(VERSION)" ./scripts/buildversion.sh $(OUT)vgaversion.c VAR16
        $(Q)$(CC) $(CFLAGS16) -c $(OUT)vgaversion.c -o $(OUT)vgaversion.o
        $(Q)$(LD) --gc-sections -T $(OUT)vgasrc/vgalayout.lds $(OUT)vgaccode16.o $(OUT)vgaentry.o $(OUT)vgaversion.o -o $@

diff --git a/scripts/buildversion.sh b/scripts/buildversion.sh
index e5ce96c..516aff5 100755
--- a/scripts/buildversion.sh
+++ b/scripts/buildversion.sh
@@ -4,14 +4,18 @@ OUTFILE="$1"
 VAR16MODE="$2"

# Extract version info
-if [ -d .git -o -f .git ]; then
-    VERSION="`git describe --tags --long --dirty`"
-elif [ -f .version ]; then
-    VERSION="`cat .version`"
+if [ -z "$BUILD_VERSION" ]; then
+    if [ -d .git -o -f .git ]; then
+        VERSION="`git describe --tags --long --dirty`"
+    elif [ -f .version ]; then
+        VERSION="`cat .version`"
+    else
+        VERSION="?"
+    fi
+    VERSION="${VERSION}-`date +"%Y%m%d_%H%M%S"`-`hostname`"
 else
-    VERSION="?"
+    VERSION="$BUILD_VERSION"
 fi
-VERSION="${VERSION}-`date +"%Y%m%d_%H%M%S"`-`hostname`"
 echo "Version: ${VERSION}"

# Build header file

[PATCH] buildversion.h: use the last git commit as timesource if tree is clean

[Gerd Hoffmann]

On Mi, 2015-06-03 at 19:32 -0400, Kevin O'Connor wrote:
> On Thu, Jun 04, 2015 at 01:25:52AM +0200, Alexander Couzens wrote:
> > Sorry for the noise, I missed the last patch from Gerd Hoffmann
> > build: add opt-out for timestamp and hostname.
> >
> > Please ignore mine if we are merging that one.
>
> Gerd, it doesn't look like you pushed your "add opt-out" patch.  Are
> you okay with the patch below instead?

Fine with me.

cheers,
  Gerd

[PATCH] buildversion.h: use the last git commit as timesource if tree is clean

[Alexander Couzens]

> Fine with me.

can we merge it?
got a coreboot patch ready to create reproducible builds.

[PATCH] buildversion.h: use the last git commit as timesource if tree is clean

[Peter Stuge]

Kevin O'Connor wrote:
> If the new "VERSION=" parameter is not provided then the default build
> version remains unchanged.

I prefer Alex' patch, because it defaults to reproducible but
provides more information if the tree has been modified, all
automatically.

A user parameter like your VERSION *on top of that* is fine, but why
not have reproducible by default like with Alex' patch when that
doesn't really lose any information?

//Peter

[PATCH] buildversion.h: use the last git commit as timesource if tree is clean

[Kevin O'Connor]

On Fri, Jun 05, 2015 at 06:41:30PM +0200, Peter Stuge wrote:
> Kevin O'Connor wrote:
> > If the new "VERSION=" parameter is not provided then the default build
> > version remains unchanged.
>
> I prefer Alex' patch, because it defaults to reproducible but
> provides more information if the tree has been modified, all
> automatically.
>
> A user parameter like your VERSION *on top of that* is fine, but why
> not have reproducible by default like with Alex' patch when that
> doesn't really lose any information?

I find value in the hostname and date on trouble reports - it helps
determine if the binary was built by a distribution and it can help
backtrack to the gcc/binutils/etc. versions.

I find value in the timestamp during debug cycles as a means of
verifying I'm running the correct code during a test run.

So, I'm against changing the default to lose this information - either
in the debault SeaBIOS build or the default coreboot build.

-Kevin

[PATCH] buildversion.h: use the last git commit as timesource if tree is clean

[Peter Stuge]

Kevin O'Connor wrote:
> > > If the new "VERSION=" parameter is not provided then the default build
> > > version remains unchanged.
> >
> > I prefer Alex' patch, because it defaults to reproducible but
> > provides more information if the tree has been modified, all
> > automatically.
> >
> > A user parameter like your VERSION *on top of that* is fine, but why
> > not have reproducible by default like with Alex' patch when that
> > doesn't really lose any information?
>
> I find value in the hostname and date on trouble reports - it helps
> determine if the binary was built by a distribution and it can help
> backtrack to the gcc/binutils/etc. versions.

Aha! Maybe we can do one even better then - and include explicit
information about the gcc/binutils/etc. versions instead of something
implicit in the hope that it helps backtrack. Having explicit
toolchain information is valuable also for reproducible builds.

Build date - sorry, I'm not convinced. This is another heuristic.
Build date together with toolchain is not a good unique identifier.

How about (calculating and?) showing a simple checksum of the binary
at runtime instead?

My point is that you and reproducible builds actually want the same
thing, but that your chosen method makes things more difficult for
reproducible builds, and I think we can find a method that works for
both.
> I find value in the timestamp during debug cycles as a means of
> verifying I'm running the correct code during a test run.

Maybe you agree that this is a corner case which could warrant a
little special effort, e.g. a new make debug.bin target, given the
benefits of having the default build be reproducible?
> So, I'm against changing the default to lose this information -
> either in the debault SeaBIOS build or the default coreboot build.

But only because the current suggestions mean you lose information,
right? Not per se?

Thanks

//Peter

[PATCH] buildversion.h: use the last git commit as timesource if tree is clean

[Kevin O'Connor]

On Mon, Jun 08, 2015 at 08:58:52PM +0200, Peter Stuge wrote:
> Kevin O'Connor wrote:
> > > > If the new "VERSION=" parameter is not provided then the default build
> > > > version remains unchanged.
> > >
> > > I prefer Alex' patch, because it defaults to reproducible but
> > > provides more information if the tree has been modified, all
> > > automatically.
> > >
> > > A user parameter like your VERSION *on top of that* is fine, but why
> > > not have reproducible by default like with Alex' patch when that
> > > doesn't really lose any information?
> >
> > I find value in the hostname and date on trouble reports - it helps
> > determine if the binary was built by a distribution and it can help
> > backtrack to the gcc/binutils/etc. versions.
>
> Aha! Maybe we can do one even better then - and include explicit
> information about the gcc/binutils/etc. versions instead of something
> implicit in the hope that it helps backtrack. Having explicit
> toolchain information is valuable also for reproducible builds.
>
> Build date - sorry, I'm not convinced. This is another heuristic.
> Build date together with toolchain is not a good unique identifier.
>
> How about (calculating and?) showing a simple checksum of the binary
> at runtime instead?
>
> My point is that you and reproducible builds actually want the same
> thing, but that your chosen method makes things more difficult for
> reproducible builds, and I think we can find a method that works for
> both.

If someone can submit a patch that provides decent information on the
toolchain then I'm certainly open to using it instead.  The toolchain
is quite varied though (binutils, gcc, make, shell, python, etc.) so
it's not an easy task.

A checksum doesn't help me - it can demonstrate something is different
in the toolchain, but gives no clue on what is different.
> > I find value in the timestamp during debug cycles as a means of
> > verifying I'm running the correct code during a test run.
>
> Maybe you agree that this is a corner case which could warrant a
> little special effort, e.g. a new make debug.bin target, given the
> benefits of having the default build be reproducible?

Agreed.
> > So, I'm against changing the default to lose this information -
> > either in the debault SeaBIOS build or the default coreboot build.
>
> But only because the current suggestions mean you lose information,
> right? Not per se?

Yes.

-Kevin

[PATCH] buildversion.h: use the last git commit as timesource if tree is clean

[Gerd Hoffmann]

Hi,
> > How about (calculating and?) showing a simple checksum of the binary
> > at runtime instead?
> If someone can submit a patch that provides decent information on the
> toolchain then I'm certainly open to using it instead.  The toolchain
> is quite varied though (binutils, gcc, make, shell, python, etc.) so
> it's not an easy task.

Most problems come from gcc & binutils though, so
  "gcc --version | head -n 1"
and
  "ld --version | head -n 1"
would be a good start.
> A checksum doesn't help me - it can demonstrate something is different
> in the toolchain, but gives no clue on what is different.

Indeed, I'd include the plain strings.  Then probably print at
debuglevel 1 so they don't clutter the screen but are in the logs.

cheers,
  Gerd

[PATCH] buildversion.h: use the last git commit as timesource if tree is clean

[Kevin O'Connor]

On Wed, Jun 03, 2015 at 07:32:51PM -0400, Kevin O'Connor wrote:
> On Thu, Jun 04, 2015 at 01:25:52AM +0200, Alexander Couzens wrote:
> > Sorry for the noise, I missed the last patch from Gerd Hoffmann
> > build: add opt-out for timestamp and hostname.
> >
> > Please ignore mine if we are merging that one.
>
> Gerd, it doesn't look like you pushed your "add opt-out" patch.  Are
> you okay with the patch below instead?  It supports a "make
> VERSION=xyz" build option that gives the user explicit control of the
> contents of the version string.

FYI, I pushed this patch.

-Kevin