Put the commit first in the build stamp, and compare only that

#809 gave the stamp a second half and left the comparison reading the whole
string. A canary reports `JingMatrix-Vector-93d66473` and no release SHA starts
with that, so `divergesFrom` was true for the very release the reader had just
flashed: the install bar warned "same number, other build", the versions sheet
coloured that row divergent, and since installed is sameNumber && !diverged, no
row was ever marked as installed. The `-dirty` branch it still tested had become
unreachable in the same commit.

The stamp now leads with the commit -- `93d66473-JingMatrix-Vector` -- so
reading it back is a prefix and nothing more, whatever hyphens a repository or a
machine turns out to have in its name. A modified tree is marked `+` rather than
`-`, in semver's sense of build metadata: after a `-` is a repository holding
this exact commit, after a `+` are changes no repository holds. Without that
distinction a hand-built framework would claim to be the release it was merely
started from.

`buildStamp` takes one apart, and `isCommit` compares as a prefix in either
direction, since a stamp carries git's short form and a release the full SHA.
Where a build was made is deliberately not compared: a fork building the same
commit builds the same code. A stamp that names no commit -- "unknown", or the
shape published between #809 and here -- is "I cannot tell" rather than
divergence, so the canaries already flashed claim neither.

On the status page the commit keeps the size it is read at and the rest is set
smaller and muted, since it is two thirds of the characters and almost never
what the row is looked up for. Copying the page still yields the whole stamp;
de-emphasising it must not shorten it.

Author: JingMatrix

build.gradle.kts

@@ -61,18 +61,26 @@ abstract class GitLatestTagValueSource : ValueSource<String, ValueSourceParamete
 }
 
 /**
- * Which build this is, in one string: where it was built and from what commit.
+ * Which build this is, in one string: what commit it came from and where it was built.
  *
  * The version code is the commit count on origin/master, so every branch build carries master's
  * number: a build flashed from a feature branch and one flashed from master both report "v2.0
  * (3052)" and cannot be told apart on the device. That is not hypothetical — it cost a real
  * investigation to establish which of the two was installed.
  *
- * Two shapes, because "where did this binary come from" has two different answers:
+ * **The commit always comes first, and what follows always says where.** Both readers of this string
+ * want the commit and nothing else — the manager compares it against the SHA a release was cut from
+ * to answer "am I running this build", and the status page sets it in a type the rest is not given
+ * — and for both it is now simply the head of the string. No rule has to work out which part is
+ * which, so none can get it wrong when a repository or a machine turns out to have a hyphen in its
+ * name.
  *
- * *On GitHub Actions*, `JingMatrix-Vector-93d66473` — the repository the code came from, then the
- * commit. Forks build the same code from the same commits, so the hash alone identifies a
- * *revision* and not a *build*, and a fork's artifact was indistinguishable from ours.
+ * The separator says which kind of "where" follows, because they mean opposite things about whether
+ * the commit describes the binary:
+ *
+ * *On GitHub Actions*, `93d66473-JingMatrix-Vector` — the commit, then the repository the code came
+ * from. Forks build the same code from the same commits, so the hash alone identifies a *revision*
+ * and not a *build*, and a fork's artifact was indistinguishable from ours.
  *
  * Both halves are the *head* ones, and neither is read from the environment GitHub sets by default,
  * because on a pull request both defaults name something other than the code that was built:
@@ -91,11 +99,17 @@ abstract class GitLatestTagValueSource : ValueSource<String, ValueSourceParamete
  * when there is not — outside a pull request the two agree anyway. Absent either, this falls back to
  * what the environment says and to `HEAD`, which covers a workflow too old to set them.
  *
- * *Locally*, the bare `93d66473`, or `93d66473-thinkpad` when the tree has uncommitted changes.
- * That build corresponds to no commit at all, so naming the commit alone would name something the
- * binary does not match — and whoever is looking at it is far better served by the machine it was
- * built on than by the word "dirty", since a device that has a hand-built framework on it usually
- * has exactly one candidate author.
+ * *Locally*, the bare `93d66473`, or `93d66473+thinkpad` when the tree has uncommitted changes. That
+ * build corresponds to no commit at all, so naming the commit alone would name something the binary
+ * does not match — and whoever is looking at it is far better served by the machine it was built on
+ * than by the word "dirty", since a device that has a hand-built framework on it usually has exactly
+ * one candidate author.
+ *
+ * `+` rather than `-` for that one, in semver's sense of build metadata: after a `-` is a repository
+ * that holds this exact commit, and after a `+` is a change that is in no repository at all. That is
+ * the one distinction a reader of the stamp cannot afford to lose — a modified build matches no
+ * release, and read as a CI build it would claim to be the release it was merely started from.
+ * Neither character can occur inside a host name or an `owner/repo`, so the two shapes stay apart.
  *
  * The dirty check is deliberately not applied on CI. The workflow's own "Write key" step appends
  * the signing credentials to the tracked `gradle.properties` before Gradle starts, so every master
@@ -165,7 +179,7 @@ abstract class GitCommitHashValueSource : ValueSource<String, GitCommitHashValue
         val repository = parameters.buildRepository.getOrElse("")
         if (repository.isBlank()) {
             val dirty = capture("git", "status", "--porcelain", "--untracked-files=no") != null
-            return if (dirty) "$head-${hostname()}" else head
+            return if (dirty) "$head+${hostname()}" else head
         }
 
         // The pushed commit is an ancestor of the merge that was checked out, so it is in the
@@ -174,7 +188,7 @@ abstract class GitCommitHashValueSource : ValueSource<String, GitCommitHashValue
         val pushed = parameters.buildCommit.getOrElse("").takeIf { it.isNotBlank() }
         val short =
             pushed?.let { capture("git", "rev-parse", "--short", it) ?: it.take(head.length) } ?: head
-        return repository.replace('/', '-') + "-" + short
+        return short + "-" + repository.replace('/', '-')
     }
 }
 

manager/README.md

@@ -86,8 +86,11 @@ asserting the old meaning until someone notices, which can be a long time.
 ```
 
 The version code is `git rev-list --count refs/remotes/origin/master`, so a branch build and a
-master build can share one; `module.prop` carries the commit, marked `-dirty` when the tree was not
-clean, which is often the only way to tell two builds apart on a device.
+master build can share one; `module.prop` and the status page carry a build stamp, which is often
+the only way to tell two builds apart on a device. It names where the build came from as well as
+what it was built from. The commit leads and what follows says where: `93d66473-JingMatrix-Vector`
+for a CI build, the bare `93d66473` for a local one, and `93d66473+thinkpad` — the machine that made
+it — when the tree was not clean.
 
 Debug builds add a second launcher activity — a demo mode with scripted device states, in
 `src/debug` and absent from release builds, so it cannot be used to make a release report a healthy

manager/src/main/kotlin/org/matrix/vector/manager/data/model/BuildStamp.kt

@@ -0,0 +1,74 @@
+package org.matrix.vector.manager.data.model
+
+/**
+ * A build stamp, taken apart: which commit a build came from, and where it was built.
+ *
+ * The framework and the manager both report one — `BuildConfig.VERSION_HASH`, and
+ * `getFrameworkCommit()` across the binder — because the version code cannot tell two builds apart:
+ * it is the commit count on origin/master, so every branch build at the same depth wears the same
+ * number as the official build it was never made from.
+ *
+ * Only the commit is taken out. Where a build was made is for a person to read, and it is exactly
+ * the rest of the string — a caller that wants it takes what follows the commit, separator and all.
+ *
+ * @property commit the commit the build was made from, abbreviated as git abbreviated it, or null
+ *   when the stamp names none — "unknown" from a build made outside a git checkout, and anything
+ *   else this does not recognise. That is a third answer, distinct from "these differ".
+ * @property modified whether the tree had uncommitted changes. Such a build was made from no commit
+ *   at all: [commit] names the one it departed from, not one the binary corresponds to.
+ */
+data class BuildStamp(val commit: String?, val modified: Boolean) {
+
+    /**
+     * Whether this build and [other] were made from the same commit.
+     *
+     * A modified tree matches nothing, including itself. Otherwise it is a prefix test in either
+     * direction, because the two sides need not be abbreviated to the same length: a stamp carries
+     * git's short form and a GitHub release carries the full SHA.
+     *
+     * **Where the build was made is deliberately not compared.** A fork building the same commit
+     * builds the same code; the repository is in the stamp to identify a *binary* to someone
+     * reading a bug report, which is a different question from "is this the build that release
+     * published".
+     */
+    fun isCommit(other: String?): Boolean {
+        if (modified || other == null) return false
+        val ours = commit ?: return false
+        return other.startsWith(ours) || ours.startsWith(other)
+    }
+}
+
+/**
+ * Reads a build stamp.
+ *
+ * The commit leads, always, and the separator says what follows it — `-` a repository that holds
+ * this exact commit, `+` a machine that holds changes no repository does. So the commit is the head
+ * and there is nothing to guess: neither character can occur inside a host name or an `owner/repo`,
+ * and a hyphen inside either is harmless because everything after the first one is the origin.
+ *
+ * - `93d66473` — a local build of a clean tree.
+ * - `93d66473-JingMatrix-Vector` — CI, from `JingMatrix/Vector`.
+ * - `93d66473+thinkpad` — a local build with uncommitted changes, named by the machine that made it.
+ *
+ * A stamp that does not start with a commit yields a null one, which everywhere it is asked means
+ * "I cannot tell" rather than "these differ". That covers the shape shipped between #809 and this
+ * change, `JingMatrix-Vector-93d66473`, which is what the canaries already on people's devices
+ * carry: they are shown as they were recorded and claimed to be neither installed nor divergent,
+ * which is the truthful answer for a build whose stamp this cannot read.
+ */
+fun buildStamp(reported: String): BuildStamp {
+    val stamp = reported.trim()
+    val head = stamp.takeWhile { it != '-' && it != '+' }
+    if (!head.isAbbreviatedSha()) return BuildStamp(null, modified = false)
+    return BuildStamp(commit = head, modified = stamp.getOrNull(head.length) == '+')
+}
+
+/**
+ * Whether this could be a commit as git abbreviates one.
+ *
+ * Seven is git's own floor for an abbreviation and forty is a full SHA. Lower case only, which git
+ * emits and which is what keeps the word "unknown" — a build made where git could not be asked —
+ * from being mistaken for one.
+ */
+private fun String.isAbbreviatedSha(): Boolean =
+    length in 7..40 && all { it.isDigit() || it in 'a'..'f' }

manager/src/main/kotlin/org/matrix/vector/manager/data/repository/FrameworkUpdateRepository.kt

@@ -5,6 +5,7 @@ import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.asStateFlow
 import org.matrix.vector.manager.data.github.FrameworkRelease
 import org.matrix.vector.manager.data.github.GitHubRepository
+import org.matrix.vector.manager.data.model.buildStamp
 
 /**
  * Whether a newer build of the framework exists, and which one this reader may be offered.
@@ -70,7 +71,12 @@ class FrameworkUpdateRepository(private val github: GitHubRepository) {
  */
 data class FrameworkUpdateState(
     val installedVersionCode: Long = 0,
-    /** The commit the running daemon was built from, when it recorded one. */
+    /**
+     * The build stamp the running daemon reports, when it recorded one.
+     *
+     * Not a bare hash: it names where the build came from as well as what commit it was made from,
+     * in one of the shapes [buildStamp] reads. Nothing here compares it as a string.
+     */
     val installedCommit: String? = null,
     val available: FrameworkRelease? = null,
     /** Every release on this channel, newest first — including ones older than the installed one. */
@@ -93,12 +99,16 @@ enum class ReleaseDirection {
  * Only askable when both sides recorded a commit: the canaries carry a SHA, a hand-made release
  * carries a branch name, and a build made before this existed carries nothing. "I cannot tell" is a
  * third answer and is reported as false rather than as divergence.
+ *
+ * What the framework reports is a *build stamp*, not a bare hash, so the commit is read out of it
+ * before anything is compared. Comparing the whole stamp is what #809 left behind: a CI stamp
+ * carries the repository as well, no release SHA matches that, and so every canary reader was told
+ * they were running "same number, other build" against the very release they had flashed, with no
+ * row anywhere marked as installed.
  */
 fun FrameworkUpdateState.divergesFrom(release: FrameworkRelease?): Boolean {
     if (release == null || release.versionCode != installedVersionCode) return false
-    val mine = installedCommit ?: return false
-    val theirs = release.commit ?: return false
-    // A dirty build matches nothing by definition — it was not built from any commit.
-    if (mine.endsWith("-dirty")) return true
-    return !theirs.startsWith(mine)
+    val mine = buildStamp(installedCommit ?: return false)
+    if (mine.commit == null || release.commit == null) return false
+    return !mine.isCommit(release.commit)
 }

manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/home/HomeViewModel.kt

@@ -47,13 +47,14 @@ data class FrameworkStatus(
     val sepolicyLoaded: Boolean = false,
     val systemServerInjected: Boolean = false,
     /**
-     * Which build the running framework is: where it came from and from what commit.
+     * Which build the running framework is: what commit it came from and where it was built.
      *
      * The version code is a commit count, so it cannot tell a branch build from the official build
      * of the same depth — and the framework and the manager are flashed separately, so they are
      * not always the same build. Naming both is the difference between a bug report that can be
-     * placed and one that cannot. A CI build reads `JingMatrix-Vector-93d66473`, a clean local one
-     * the bare hash, and a local build from a modified tree adds the machine that made it.
+     * placed and one that cannot. The commit leads: a CI build reads `93d66473-JingMatrix-Vector`,
+     * a clean local one the bare hash, and a local build from a modified tree marks the machine
+     * that made it with a `+`. Taken apart by `buildStamp`; nothing compares it as a string.
      */
     val commit: String? = null,
 ) {