New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop build date #336

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
5 participants
@bmwiedemann

bmwiedemann commented Sep 26, 2017

and build user and build host
in order to make builds reproducible.
See https://reproducible-builds.org/ for why this is good.

Made this as an alternative approach to #332

Drop build date
and build user and build host
in order to make builds reproducible.
See https://reproducible-builds.org/ for why this is good.
@abartlet

This comment has been minimized.

Show comment
Hide comment
@abartlet

abartlet Sep 26, 2017

Member
Member

abartlet commented Sep 26, 2017

@nicowilliams

This comment has been minimized.

Show comment
Hide comment
@nicowilliams

nicowilliams Sep 26, 2017

Contributor

@abartlet So do I. Is there any reason to keep any of HOST, USER, or DATE ever? I'd rather remove them altogether. The only things I want are the actual version, maybe the branch, closest tag, HEAD commit, and/or indication of whether the clone built in is dirty.

Contributor

nicowilliams commented Sep 26, 2017

@abartlet So do I. Is there any reason to keep any of HOST, USER, or DATE ever? I'd rather remove them altogether. The only things I want are the actual version, maybe the branch, closest tag, HEAD commit, and/or indication of whether the clone built in is dirty.

@abartlet

This comment has been minimized.

Show comment
Hide comment
@abartlet

abartlet Sep 26, 2017

Member
Member

abartlet commented Sep 26, 2017

@nicowilliams nicowilliams self-assigned this Sep 27, 2017

@nicowilliams

This comment has been minimized.

Show comment
Hide comment
@nicowilliams

nicowilliams Sep 27, 2017

Contributor

I'll push my own version of this. I'll just use various git things. I'm testing this:

diff --git a/configure.ac b/configure.ac
index 806b639..4d4237f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -667,7 +667,7 @@ cat > include/newversion.h.in <<EOF
 #ifndef VERSION_HIDDEN
 #define VERSION_HIDDEN
 #endif
-VERSION_HIDDEN const char *heimdal_long_version = "@([#])\$Version: $PACKAGE_STRING by @USER@ on @HOST@ ($host) @DATE@ \$";
+VERSION_HIDDEN const char *heimdal_long_version = "@([#])\$Version: $PACKAGE_STRING @BRANCH@ @TAG@ ($host) @COMMIT@ \$";
 VERSION_HIDDEN const char *heimdal_version = "AC_PACKAGE_STRING";
 EOF

@@ -676,9 +676,19 @@ if test -f include/version.h && cmp -s include/newversion.h.in include/version.h
        rm -f include/newversion.h.in
 else
        echo "creating include/version.h"
-       User=${USER-${LOGNAME}}
-       Host=`(hostname || uname -n || echo unknown) 2>/dev/null | sed 1q`
-       Date=`date`
+        if test -d "$srcdir/.git"; then
+            GitCommit=`git rev-parse HEAD`
+            GitBranch=`git rev-parse --abbrev-ref HEAD`
+            if test "x$GitBranch" = master; then
+                GitDesc=`git describe --all --dirty`
+            else
+                GitDesc=`git describe --tags --match 'heimdal-*' --dirty`
+            fi
+        else
+            GitCommit='<commit-unknown>'
+            GitBranch='<branch-unknown>'
+            GitDesc='<tag-unknown>'
+        fi
        mv -f include/newversion.h.in include/version.h.in
-       sed -e "s/@USER@/$User/" -e "s/@HOST@/$Host/" -e "s/@DATE@/$Date/" include/version.h.in > include/version.h
+       sed -e "s;@BRANCH@;$GitBranch;" -e "s;@TAG@;$GitDesc;" -e "s;@COMMIT@;$GitCommit;" include/version.h.in > include/version.h
 fi

Similarly for cf/krb-version.m4. Why do we have that duplication?? We don't use the AC_KRB_VERSION macro... I'll try removing it.

Contributor

nicowilliams commented Sep 27, 2017

I'll push my own version of this. I'll just use various git things. I'm testing this:

diff --git a/configure.ac b/configure.ac
index 806b639..4d4237f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -667,7 +667,7 @@ cat > include/newversion.h.in <<EOF
 #ifndef VERSION_HIDDEN
 #define VERSION_HIDDEN
 #endif
-VERSION_HIDDEN const char *heimdal_long_version = "@([#])\$Version: $PACKAGE_STRING by @USER@ on @HOST@ ($host) @DATE@ \$";
+VERSION_HIDDEN const char *heimdal_long_version = "@([#])\$Version: $PACKAGE_STRING @BRANCH@ @TAG@ ($host) @COMMIT@ \$";
 VERSION_HIDDEN const char *heimdal_version = "AC_PACKAGE_STRING";
 EOF

@@ -676,9 +676,19 @@ if test -f include/version.h && cmp -s include/newversion.h.in include/version.h
        rm -f include/newversion.h.in
 else
        echo "creating include/version.h"
-       User=${USER-${LOGNAME}}
-       Host=`(hostname || uname -n || echo unknown) 2>/dev/null | sed 1q`
-       Date=`date`
+        if test -d "$srcdir/.git"; then
+            GitCommit=`git rev-parse HEAD`
+            GitBranch=`git rev-parse --abbrev-ref HEAD`
+            if test "x$GitBranch" = master; then
+                GitDesc=`git describe --all --dirty`
+            else
+                GitDesc=`git describe --tags --match 'heimdal-*' --dirty`
+            fi
+        else
+            GitCommit='<commit-unknown>'
+            GitBranch='<branch-unknown>'
+            GitDesc='<tag-unknown>'
+        fi
        mv -f include/newversion.h.in include/version.h.in
-       sed -e "s/@USER@/$User/" -e "s/@HOST@/$Host/" -e "s/@DATE@/$Date/" include/version.h.in > include/version.h
+       sed -e "s;@BRANCH@;$GitBranch;" -e "s;@TAG@;$GitDesc;" -e "s;@COMMIT@;$GitCommit;" include/version.h.in > include/version.h
 fi

Similarly for cf/krb-version.m4. Why do we have that duplication?? We don't use the AC_KRB_VERSION macro... I'll try removing it.

@nicowilliams

This comment has been minimized.

Show comment
Hide comment
@nicowilliams

nicowilliams Sep 27, 2017

Contributor

One thing that's clear is that we need a heimdal-8.0pre tag so we don't get this:

#ifndef VERSION_HIDDEN
#define VERSION_HIDDEN
#endif
VERSION_HIDDEN const char *heimdal_long_version = "@(#)$Version: Heimdal 7.99.1 master heimdal-1.5pre2-1992-gaef3843-dirty (x86_64-pc-linux-gnu) aef3843b552c4694cdfe99cca2896579561a1a2c $";
VERSION_HIDDEN const char *heimdal_version = "Heimdal 7.99.1";
Contributor

nicowilliams commented Sep 27, 2017

One thing that's clear is that we need a heimdal-8.0pre tag so we don't get this:

#ifndef VERSION_HIDDEN
#define VERSION_HIDDEN
#endif
VERSION_HIDDEN const char *heimdal_long_version = "@(#)$Version: Heimdal 7.99.1 master heimdal-1.5pre2-1992-gaef3843-dirty (x86_64-pc-linux-gnu) aef3843b552c4694cdfe99cca2896579561a1a2c $";
VERSION_HIDDEN const char *heimdal_version = "Heimdal 7.99.1";

nicowilliams added a commit to nicowilliams/heimdal that referenced this pull request Sep 27, 2017

@nicowilliams

This comment has been minimized.

Show comment
Hide comment
@nicowilliams

nicowilliams Sep 27, 2017

Contributor

What do you think of this version of these changes:

https://github.com/heimdal/heimdal/compare/master...nicowilliams:master?expand=1

?

Contributor

nicowilliams commented Sep 27, 2017

What do you think of this version of these changes:

https://github.com/heimdal/heimdal/compare/master...nicowilliams:master?expand=1

?

@jaltman

This comment has been minimized.

Show comment
Hide comment
@jaltman

jaltman Sep 27, 2017

Member

We recently uncovered a situation where a modified binary Heimdal distribution which included malware was distributed via sourceforge. One of the details that made it obvious that the build details (timestamp, host, user, etc) were present in the build. That evidence was important when it came to convincing sourceforge to take down the binaries.

My preference is for the default build to include these details in the version string and provide an option that can be specified for reproducible builds

Member

jaltman commented Sep 27, 2017

We recently uncovered a situation where a modified binary Heimdal distribution which included malware was distributed via sourceforge. One of the details that made it obvious that the build details (timestamp, host, user, etc) were present in the build. That evidence was important when it came to convincing sourceforge to take down the binaries.

My preference is for the default build to include these details in the version string and provide an option that can be specified for reproducible builds

@kaduk

This comment has been minimized.

Show comment
Hide comment
@kaduk

kaduk Sep 27, 2017

Contributor

IIUC the recommended "option" to enable the reproducible build is to detect that the SOURCE_DATE_EPOCH environment variable is set (and use that as the datestamp if needed).

Contributor

kaduk commented Sep 27, 2017

IIUC the recommended "option" to enable the reproducible build is to detect that the SOURCE_DATE_EPOCH environment variable is set (and use that as the datestamp if needed).

@nicowilliams

This comment has been minimized.

Show comment
Hide comment
@nicowilliams

nicowilliams Sep 28, 2017

Contributor

I see. Mind you, the git info my variant of this patch uses... would have revealed the sourceforge copy to be illegitimate too.

Contributor

nicowilliams commented Sep 28, 2017

I see. Mind you, the git info my variant of this patch uses... would have revealed the sourceforge copy to be illegitimate too.

nicowilliams added a commit to nicowilliams/heimdal that referenced this pull request Sep 29, 2017

nicowilliams added a commit to nicowilliams/heimdal that referenced this pull request Sep 29, 2017

@nicowilliams

This comment has been minimized.

Show comment
Hide comment
@nicowilliams

nicowilliams Sep 29, 2017

Contributor

I've pushed another version: nicowilliams@6d27e00

This one uses these env vars: SOURCE_HOST, SOURCE_USER, and SOURCE_DATE_EPOCH. Only SOURCE_DATE_EPOCH appears to be "standard". I'm still adding git branch/tag/commit hash information when building from a git clone.

Contributor

nicowilliams commented Sep 29, 2017

I've pushed another version: nicowilliams@6d27e00

This one uses these env vars: SOURCE_HOST, SOURCE_USER, and SOURCE_DATE_EPOCH. Only SOURCE_DATE_EPOCH appears to be "standard". I'm still adding git branch/tag/commit hash information when building from a git clone.

@nicowilliams

This comment has been minimized.

Show comment
Hide comment
@nicowilliams

nicowilliams Oct 5, 2017

Contributor

I've pushed an alternative version.

Contributor

nicowilliams commented Oct 5, 2017

I've pushed an alternative version.

@nicowilliams

This comment has been minimized.

Show comment
Hide comment
@nicowilliams

nicowilliams Oct 5, 2017

Contributor

Thanks!

Contributor

nicowilliams commented Oct 5, 2017

Thanks!

@bmwiedemann

This comment has been minimized.

Show comment
Hide comment
@bmwiedemann

bmwiedemann Oct 5, 2017

@nicowilliams you used date -d which only works with GNU date, not BSD date, which is why #332 used a more complex date call ... or is BSD not a supported platform?

bmwiedemann commented Oct 5, 2017

@nicowilliams you used date -d which only works with GNU date, not BSD date, which is why #332 used a more complex date call ... or is BSD not a supported platform?

@nicowilliams

This comment has been minimized.

Show comment
Hide comment
@nicowilliams

nicowilliams Oct 5, 2017

Contributor

@bmwiedemann Oh, date -u -d "@$SOURCE_DATE_EPOCH" 2>/dev/null || date -u -r "$SOURCE_DATE_EPOCH" 2>/dev/null || date -u? Sorry, I just simply missed that. I'll fix it!

Contributor

nicowilliams commented Oct 5, 2017

@bmwiedemann Oh, date -u -d "@$SOURCE_DATE_EPOCH" 2>/dev/null || date -u -r "$SOURCE_DATE_EPOCH" 2>/dev/null || date -u? Sorry, I just simply missed that. I'll fix it!

@nicowilliams

This comment has been minimized.

Show comment
Hide comment
@nicowilliams

nicowilliams Oct 5, 2017

Contributor

Illumos has nothing like -r/-d. Sigh.

Contributor

nicowilliams commented Oct 5, 2017

Illumos has nothing like -r/-d. Sigh.

@nicowilliams

This comment has been minimized.

Show comment
Hide comment
@nicowilliams

nicowilliams Oct 5, 2017

Contributor

I've pushed a fix. Thanks for noticing this!

Contributor

nicowilliams commented Oct 5, 2017

I've pushed a fix. Thanks for noticing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment