From f72650d2de11cfcbd123d22f9a603fbf9eab3ac6 Mon Sep 17 00:00:00 2001
From: UdjinM6 <UdjinM6@users.noreply.github.com>
Date: Fri, 12 Jan 2024 06:43:42 +0300
Subject: [PATCH] feat: Set client version for non-release binaries and version
 in guix based on git tags (#5653)

## Issue being fixed or feature implemented
Client version string is inconsistent. Building `v20.0.0-beta.8` tag
locally produces binaries that report `v20.0.0-beta.8` version but
binaries built in guix would report
`v20.0.0rc1-g3e732a952226a20505f907e4fd9b3fdbb14ea5ee` instead. Building
any commit after `v20.0.0-beta.8` locally would result in versions like
`v20.0.0rc1-8c94153d2497` which is close but it's still yet another
format. And both versions with `rc1` in their names are confusing cause
you'd expect them to mention `beta.8` instead maybe (or is it just me?
:D ).

## What was done?
Change it so that the version string would look like this:
on tag: ~`v20.0.0-beta.8-dev` or `v20.0.0-beta.8-gitarc`~
`v20.0.0-beta.8`
post-tag: ~`v20.0.0-beta.8-1-gb837e08164-gitarc`~
`v20.0.0-beta.8-1-gb837e08164`

post-tag format is
`recent tag`-`commits since that tag`-`g+12 chars of commit hash`-`dirty
(optional)` ~-`dev or gitarc`~

~`dev`/`gitarc` suffixes should help avoiding confusion with the release
versions and they also indicate the way non-release binaries were
built.~

Note that release binaries do not use any of this, they still use
`PACKAGE_VERSION` from `configure` like before.

Also, `CLIENT_VERSION_RC` is no longer used in this setup so it was
removed.

Few things aren't clear to me yet:
1. Version bump in `configure.ac` no longer affects the reported version
(unless it's an actual release). Are there any downsides I might be
missing?
2. Which tag should we use on `develop` once we bump version in
configure? `v21.0.0-init`? `v21.0.0-alpha1`?
3. How is it going to behave once `merge master back into develop` kind
of PR is merged? E.g. say `develop` branch is on `v21.0.0-alpha1` tag
and we merge v20.1.0 from `master` back into it. Will this bring
`v20.1.0` release tag into `develop`? Will it become the one that will
be used from that moment? If so we will probably need another tag on
`develop` every time such PR is merged e.g. `v21.0.0-alpha2` (or
whatever the next number is).

Don't think these are blockers but would like to hear thoughts from
others.

## How Has This Been Tested?
Built binaries locally, built them using guix at a specific tag and at
some commit on top of it.

## Breaking Changes
n/a

## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
---
 .github/workflows/guix-build.yml           |  1 +
 configure.ac                               |  4 ++--
 contrib/containers/guix/scripts/guix-check |  7 ++++---
 contrib/guix/libexec/codesign.sh           |  8 ++------
 contrib/shell/git-utils.bash               |  8 ++------
 doc/release-process.md                     |  3 +--
 share/genbuild.sh                          | 20 +++++--------------
 src/clientversion.cpp                      | 23 +++++++++++-----------
 8 files changed, 29 insertions(+), 45 deletions(-)

diff --git a/.github/workflows/guix-build.yml b/.github/workflows/guix-build.yml
index 9b91fb2beacd5..dbdaddcb37e8b 100644
--- a/.github/workflows/guix-build.yml
+++ b/.github/workflows/guix-build.yml
@@ -16,6 +16,7 @@ jobs:
         with:
           ref: ${{ github.event.pull_request.head.sha }}
           path: dash
+          fetch-depth: 0
 
       - name: Set up Docker Buildx
         uses: docker/setup-buildx-action@v3
diff --git a/configure.ac b/configure.ac
index bc003af93c692..9623cc6b44a88 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,13 +1,13 @@
 AC_PREREQ([2.69])
+dnl Don't forget to push a corresponding tag when updating any of _CLIENT_VERSION_* numbers
 define(_CLIENT_VERSION_MAJOR, 20)
 define(_CLIENT_VERSION_MINOR, 1)
 define(_CLIENT_VERSION_BUILD, 0)
-define(_CLIENT_VERSION_RC, 0)
 define(_CLIENT_VERSION_IS_RELEASE, false)
 define(_COPYRIGHT_YEAR, 2023)
 define(_COPYRIGHT_HOLDERS,[The %s developers])
 define(_COPYRIGHT_HOLDERS_SUBSTITUTION,[[Dash Core]])
-AC_INIT([Dash Core],m4_join([.], _CLIENT_VERSION_MAJOR, _CLIENT_VERSION_MINOR, _CLIENT_VERSION_BUILD)m4_if(_CLIENT_VERSION_RC, [0], [], [rc]_CLIENT_VERSION_RC),[https://github.com/dashpay/dash/issues],[dashcore],[https://dash.org/])
+AC_INIT([Dash Core],m4_join([.], _CLIENT_VERSION_MAJOR, _CLIENT_VERSION_MINOR, _CLIENT_VERSION_BUILD),[https://github.com/dashpay/dash/issues],[dashcore],[https://dash.org/])
 AC_CONFIG_SRCDIR([src/validation.cpp])
 AC_CONFIG_HEADERS([src/config/bitcoin-config.h])
 AC_CONFIG_AUX_DIR([build-aux])
diff --git a/contrib/containers/guix/scripts/guix-check b/contrib/containers/guix/scripts/guix-check
index e16891bc72a2e..7111f63c5b02a 100755
--- a/contrib/containers/guix/scripts/guix-check
+++ b/contrib/containers/guix/scripts/guix-check
@@ -14,15 +14,16 @@ if [[ ! -d "$WORKSPACE_PATH" ]]; then
 fi
 
 cd "$WORKSPACE_PATH"
-COMMIT_ID="$(git rev-parse --short=12 HEAD)"
+
+source "contrib/guix/libexec/prelude.bash"
 
 printf "\nBinaries:\n\n"
 ( \
-SRC_PATH_PREFIX="guix-build-${COMMIT_ID}/distsrc-" && \
+SRC_PATH_PREFIX="${VERSION_BASE}/distsrc-" && \
 sha256sum ${SRC_PATH_PREFIX}*/src/dash{d,-cli,-tx,-wallet}{,.exe} && \
 sha256sum ${SRC_PATH_PREFIX}*/src/qt/dash-qt{,.exe} && \
 sha256sum ${SRC_PATH_PREFIX}*/src/test/test_dash{,.exe} \
 ) | sort -k 2
 
 printf "\nArchives:\n\n"
-find "guix-build-${COMMIT_ID}/output" -type f | grep -v SHA256 | xargs sha256sum | sort -k 2
+find "${OUTDIR_BASE}" -type f | grep -v SHA256 | xargs sha256sum | sort -k 2
diff --git a/contrib/guix/libexec/codesign.sh b/contrib/guix/libexec/codesign.sh
index 59df8e87fac22..137dbe4427590 100755
--- a/contrib/guix/libexec/codesign.sh
+++ b/contrib/guix/libexec/codesign.sh
@@ -38,12 +38,8 @@ ACTUAL_OUTDIR="${OUTDIR}"
 OUTDIR="${DISTSRC}/output"
 
 git_head_version() {
-    local recent_tag
-    if recent_tag="$(git -C "$1" describe --exact-match HEAD 2> /dev/null)"; then
-        echo "${recent_tag#v}"
-    else
-        git -C "$1" rev-parse --short=12 HEAD
-    fi
+    recent_tag="$(git -C "$1" describe --abbrev=12 --dirty 2> /dev/null)"
+    echo "${recent_tag#v}"
 }
 
 CODESIGNATURE_GIT_ARCHIVE="${DIST_ARCHIVE_BASE}/${DISTNAME}-codesignatures-$(git_head_version "$DETACHED_SIGS_REPO").tar.gz"
diff --git a/contrib/shell/git-utils.bash b/contrib/shell/git-utils.bash
index 37bac1f38d8f0..1371c3df70c0f 100644
--- a/contrib/shell/git-utils.bash
+++ b/contrib/shell/git-utils.bash
@@ -5,10 +5,6 @@ git_root() {
 }
 
 git_head_version() {
-    local recent_tag
-    if recent_tag="$(git describe --exact-match HEAD 2> /dev/null)"; then
-        echo "${recent_tag#v}"
-    else
-        git rev-parse --short=12 HEAD
-    fi
+    recent_tag="$(git describe --abbrev=12 --dirty 2> /dev/null)"
+    echo "${recent_tag#v}"
 }
diff --git a/doc/release-process.md b/doc/release-process.md
index 5df613449bc17..10c12d3ac6b09 100644
--- a/doc/release-process.md
+++ b/doc/release-process.md
@@ -3,13 +3,12 @@ Release Process
 
 * [ ] Update translations, see [translation_process.md](https://github.com/dashpay/dash/blob/master/doc/translation_process.md#synchronising-translations).
 * [ ] Update manpages, see [gen-manpages.sh](https://github.com/dashpay/dash/blob/master/contrib/devtools/README.md#gen-manpagessh).
-* [ ] Update release candidate version in `configure.ac` (`CLIENT_VERSION_RC`)
 
 Before every minor and major release:
 
 * [ ] Update [bips.md](bips.md) to account for changes since the last release.
 * [ ] Update DIPs with any changes introduced by this release (see [this pull request](https://github.com/dashpay/dips/pull/142) for an example)
-* [ ] Update version in `configure.ac` (don't forget to set `CLIENT_VERSION_IS_RELEASE` to `true`) (don't forget to set `CLIENT_VERSION_RC` to `0`)
+* [ ] Update version in `configure.ac` (don't forget to set `CLIENT_VERSION_IS_RELEASE` to `true`)
 * [ ] Write release notes (see below)
 * [ ] Update `src/chainparams.cpp` `nMinimumChainWork` with information from the `getblockchaininfo` rpc.
 * [ ] Update `src/chainparams.cpp` `defaultAssumeValid` with information from the `getblockhash` rpc.
diff --git a/share/genbuild.sh b/share/genbuild.sh
index 4da97dce0149b..1de8039e291b5 100755
--- a/share/genbuild.sh
+++ b/share/genbuild.sh
@@ -18,27 +18,17 @@ else
     exit 1
 fi
 
-GIT_TAG=""
-GIT_COMMIT=""
+GIT_DESCRIPTION=""
 if [ "${BITCOIN_GENBUILD_NO_GIT}" != "1" ] && [ -e "$(command -v git)" ] && [ "$(git rev-parse --is-inside-work-tree 2>/dev/null)" = "true" ]; then
     # clean 'dirty' status of touched files that haven't been modified
     git diff >/dev/null 2>/dev/null
 
-    # if latest commit is tagged and not dirty, then override using the tag name
-    RAWDESC=$(git describe --abbrev=0 2>/dev/null)
-    if [ "$(git rev-parse HEAD)" = "$(git rev-list -1 $RAWDESC 2>/dev/null)" ]; then
-        git diff-index --quiet HEAD -- && GIT_TAG=$RAWDESC
-    fi
-
-    # otherwise generate suffix from git, i.e. string like "59887e8-dirty"
-    GIT_COMMIT=$(git rev-parse --short=12 HEAD)
-    git diff-index --quiet HEAD -- || GIT_COMMIT="$GIT_COMMIT-dirty"
+    # override using the tag name from git, i.e. string like "v20.0.0-beta.8-5-g99786590df6f-dirty"
+    GIT_DESCRIPTION=$(git describe --abbrev=12 --dirty 2>/dev/null)
 fi
 
-if [ -n "$GIT_TAG" ]; then
-    NEWINFO="#define BUILD_GIT_TAG \"$GIT_TAG\""
-elif [ -n "$GIT_COMMIT" ]; then
-    NEWINFO="#define BUILD_GIT_COMMIT \"$GIT_COMMIT\""
+if [ -n "$GIT_DESCRIPTION" ]; then
+    NEWINFO="#define BUILD_GIT_DESCRIPTION \"$GIT_DESCRIPTION\""
 else
     NEWINFO="// No build information available"
 fi
diff --git a/src/clientversion.cpp b/src/clientversion.cpp
index 35d02d4873fe6..6fdd302b1eb60 100644
--- a/src/clientversion.cpp
+++ b/src/clientversion.cpp
@@ -19,25 +19,26 @@ const std::string CLIENT_NAME("Dash Core");
 #include <obj/build.h>
 // The <obj/build.h>, which is generated by the build environment (share/genbuild.sh),
 // could contain only one line of the following:
-//   - "#define BUILD_GIT_TAG ...", if the top commit is tagged
-//   - "#define BUILD_GIT_COMMIT ...", if the top commit is not tagged
+//   - "#define BUILD_GIT_DESCRIPTION ...", if the top commit is not tagged
 //   - "// No build information available", if proper git information is not available
 #endif
 
-//! git will put "#define GIT_COMMIT_ID ..." on the next line inside archives. $Format:%n#define GIT_COMMIT_ID "%H"$
+//! git will put "#define ARCHIVE_GIT_DESCRIPTION ..." on the next line inside archives. $Format:%n#define ARCHIVE_GIT_DESCRIPTION "%(describe:abbrev=12)"$
 
-#ifdef BUILD_GIT_TAG
-    #define BUILD_DESC BUILD_GIT_TAG
+#if CLIENT_VERSION_IS_RELEASE
+    #define BUILD_DESC "v" PACKAGE_VERSION
     #define BUILD_SUFFIX ""
 #else
-    #define BUILD_DESC "v" PACKAGE_VERSION
-    #if CLIENT_VERSION_IS_RELEASE
+    #if defined(BUILD_GIT_DESCRIPTION)
+        // build in a cloned folder
+        #define BUILD_DESC BUILD_GIT_DESCRIPTION
+        #define BUILD_SUFFIX ""
+    #elif defined(ARCHIVE_GIT_DESCRIPTION)
+        // build in a folder from git archive
+        #define BUILD_DESC ARCHIVE_GIT_DESCRIPTION
         #define BUILD_SUFFIX ""
-    #elif defined(BUILD_GIT_COMMIT)
-        #define BUILD_SUFFIX "-" BUILD_GIT_COMMIT
-    #elif defined(GIT_COMMIT_ID)
-        #define BUILD_SUFFIX "-g" GIT_COMMIT_ID
     #else
+        #define BUILD_DESC "v" PACKAGE_VERSION
         #define BUILD_SUFFIX "-unk"
     #endif
 #endif