Public Access
Compare commits
2
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
 jaap-jan
|
b4619db8d2 | ||
|
jaap-jan
|
3d3d0bc95f |
+160
-16
@@ -147,13 +147,25 @@ jobs:
|
||||
# green, the package is cut, and the symptom arrives weeks later as clients that never update.
|
||||
#
|
||||
# -getProperty evaluates without building, so this costs a second and runs before the build.
|
||||
#
|
||||
# ◆ -t:MinVer IS NOT OPTIONAL, AND WITHOUT IT THIS STEP FAILED EVERY TAG BUILD.
|
||||
#
|
||||
# -getProperty on its own evaluates the project and runs no targets, and MinVer sets Version
|
||||
# from inside a target — so what came back was the SDK's default 1.0.0, for any tag, on any
|
||||
# commit, in a clean checkout with every tag present. The step would have failed the first
|
||||
# release ever cut and blamed fetch-depth for it, which is the one explanation that is not it.
|
||||
# Naming a target makes -getProperty report the value after that target has run.
|
||||
#
|
||||
# Never exercised, because no v* tag exists yet: this whole step is `if:` a tag ref. It is the
|
||||
# shape of bug a guard is prone to — the guard is the thing nothing tests, and it only speaks
|
||||
# on the one build nobody is watching, which is the reason this step exists at all.
|
||||
- name: the tag and the version agree
|
||||
if: startsWith(github.ref, 'refs/tags/v')
|
||||
run: |
|
||||
set -euo pipefail
|
||||
tag="${GITHUB_REF#refs/tags/v}"
|
||||
declared="$(dotnet msbuild src/DodoSSH.Client.App/DodoSSH.Client.App.csproj \
|
||||
-getProperty:Version -nologo | tr -d '[:space:]')"
|
||||
-getProperty:Version -t:MinVer -nologo | tr -d '[:space:]')"
|
||||
|
||||
if [ "$tag" != "$declared" ]; then
|
||||
echo "The tag says v$tag and MinVer computed $declared." >&2
|
||||
@@ -214,8 +226,7 @@ jobs:
|
||||
# The one build shape nothing else here exercises: a self-contained RID-specific publish. Its
|
||||
# failure mode is a restore graph or a native asset that resolves for net10.0 and not for
|
||||
# net10.0/win-x64, which nobody would see until a person was halfway through cutting a release
|
||||
# on a Windows machine. vpk can pack a Windows package from Linux; only signing needs Windows,
|
||||
# and this repository signs nothing yet, so proving the publish here is worth the minutes.
|
||||
# on a Windows machine.
|
||||
#
|
||||
# RestoreLockedMode=false for this command only, and it is not a loosened gate. The committed
|
||||
# lock files are deliberately RID-free: declaring win-x64 on the desktop head writes a
|
||||
@@ -224,20 +235,144 @@ jobs:
|
||||
# under locked mode, so the image job would fail NU1004. The gate is the locked solution
|
||||
# restore at the top of this job, which is unchanged.
|
||||
#
|
||||
# It rewrites the lock files as it goes; nothing after this step reads them, and the runner's
|
||||
# It rewrites the lock files as it goes; nothing after it reads them, and the runner's
|
||||
# checkout is thrown away. The release script does the same thing and puts them back, because
|
||||
# there the tree is somebody's working copy.
|
||||
#
|
||||
# After the tests rather than before them, so a red suite does not first spend a hundred
|
||||
# megabytes pulling a win-x64 runtime pack. main and tags only, for the same reason: a break
|
||||
# found by the person about to release is found early enough.
|
||||
- name: the windows publish still resolves
|
||||
#
|
||||
# The output directory is read by the packaging step below, so the two names have to agree.
|
||||
- name: publish the windows desktop client
|
||||
if: github.event_name != 'pull_request'
|
||||
run: >
|
||||
dotnet publish src/DodoSSH.Client.App/DodoSSH.Client.App.csproj
|
||||
--configuration Release --runtime win-x64 --self-contained true
|
||||
-p:RestoreLockedMode=false
|
||||
--output "$RUNNER_TEMP/win-x64-check"
|
||||
--output "$RUNNER_TEMP/win-x64"
|
||||
|
||||
# ◆ THE WINDOWS INSTALLER IS BUILT HERE, ON LINUX, AND IS DELIBERATELY THROWN AWAY.
|
||||
#
|
||||
# Packaging rather than only publishing, for the same reason the android job packages an APK
|
||||
# nobody installs: the failures a release is most exposed to are the ones only the packager
|
||||
# finds. vpk opens the published binaries and verifies that the main executable really calls
|
||||
# VelopackApp.Build().Run() — so a refactor that drops that call, which compiles, tests green
|
||||
# and produces an application that silently never updates itself, fails here instead of
|
||||
# shipping. A publish alone cannot see that, and the person who would otherwise see it first is
|
||||
# the one midway through a release.
|
||||
#
|
||||
# ◆ NOT WITH scripts/release-windows.ps1. That script is a person's release procedure and holds
|
||||
# things a runner must not have and must not skip — it refuses a dirty tree, insists HEAD is
|
||||
# tagged, downloads the previous release to build deltas against, and in its second phase asks
|
||||
# for the forge token. Calling it from here would mean either weakening it with CI switches or
|
||||
# having CI meet conditions that only make sense at a desk. So the packaging arguments below
|
||||
# are written out again in shell, and the duplication is accepted and stated: the pack id, the
|
||||
# title, the authors, the channel and the icon are a contract with VelopackUpdateChannel and
|
||||
# with every installed client. Change one, change the other.
|
||||
#
|
||||
# ◆ AND CROSS-COMPILING NEEDS THE OS DIRECTIVE, WHICH IS EASY TO MISS.
|
||||
#
|
||||
# vpk picks its target from the host: on Linux, plain `vpk pack` builds a Linux release and
|
||||
# refuses a Windows RID outright — "the target rid must be Linux (actually was Windows)". The
|
||||
# bracketed [win] before the verb is what turns it into a cross-compile, and it must be quoted
|
||||
# or the shell reads it as a glob matching any one of w, i and n. With it, the runner writes
|
||||
# DodoSSH.Desktop-win-Setup.exe, the portable zip, the full .nupkg and releases.win.json,
|
||||
# exactly as a Windows machine does. Only signing needs Windows tooling, and nothing here is
|
||||
# signed — see ADR 0013 rule 8 for the trigger that changes.
|
||||
#
|
||||
# ◆ NOTHING IS UPLOADED, AND NOTHING IS EVEN OFFERED FOR DOWNLOAD.
|
||||
#
|
||||
# The output goes to RUNNER_TEMP and dies with the job. That is the whole point rather than an
|
||||
# omission: ADR 0013 rule 3 puts the capability to ship somebody a build on a machine which is
|
||||
# not a runner, because Velopack clients apply what their feed serves without verifying a
|
||||
# signature. Publishing a CI-built package as a workflow artefact would not cross that line by
|
||||
# itself, and it would put an installer nobody has run in a place that invites passing it on —
|
||||
# so it is not done either. What reaches users is built, installed and walked through Phase 16
|
||||
# of docs/manual-checks.md by a person first.
|
||||
- name: package the windows desktop client
|
||||
if: github.event_name != 'pull_request'
|
||||
run: |
|
||||
set -euo pipefail
|
||||
|
||||
# vpk comes from the tool manifest, at the version committed there, which is the same one
|
||||
# the release script uses. Restored here rather than in a step of its own because this is
|
||||
# the only job that needs a tool at all.
|
||||
dotnet tool restore
|
||||
|
||||
# The same value the release script packs with, read the same way. A version vpk disagrees
|
||||
# with the assemblies about is a client that compares wrongly, so it is read from MSBuild
|
||||
# rather than reconstructed from the ref — a main build has no tag to reconstruct from
|
||||
# anyway, and MinVer's answer there carries the prerelease height that tells two of them
|
||||
# apart.
|
||||
#
|
||||
# -t:MinVer for the reason spelled out on the tag check above: without a target named,
|
||||
# -getProperty answers 1.0.0 and every package would carry the same version forever, which
|
||||
# Velopack reads as "no update available".
|
||||
version="$(dotnet msbuild src/DodoSSH.Client.App/DodoSSH.Client.App.csproj \
|
||||
-getProperty:Version -t:MinVer -nologo | tr -d '[:space:]')"
|
||||
|
||||
if [ -z "$version" ]; then
|
||||
echo "Could not read the version from MSBuild." >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# ◆ AND A FLOOR UNDER IT, BECAUSE MINVER'S PRE-FIRST-TAG ANSWER IS ONE vpk REFUSES.
|
||||
#
|
||||
# Until a v* tag exists MinVer answers 0.0.0-alpha.0.N — correct, documented, and rejected
|
||||
# outright by vpk: "Invalid package version '0.0.0-alpha.0.143', it must be >= 0.0.1". So
|
||||
# today, and on every main build until the first release, packing the true version cannot
|
||||
# work at all.
|
||||
#
|
||||
# The patch digit is lifted and the prerelease part kept, so the stand-in still sorts below
|
||||
# anything real and still differs between commits. Sound only because this package is
|
||||
# thrown away: what is being proved here is that the desktop client packages, and no client
|
||||
# will ever see this number. The release script is not given a floor and must not be — its
|
||||
# version is the one users compare against, and if MinVer ever answered 0.0.0 there the
|
||||
# right outcome is the refusal.
|
||||
packVersion="$version"
|
||||
case "$packVersion" in
|
||||
0.0.0*)
|
||||
packVersion="0.0.1${packVersion#0.0.0}"
|
||||
echo "MinVer says $version, which vpk will not pack; packaging as $packVersion."
|
||||
echo "This disappears the moment a v* tag exists, and never reaches a release."
|
||||
;;
|
||||
esac
|
||||
|
||||
releases="$RUNNER_TEMP/win-releases"
|
||||
|
||||
# --skip-updates because vpk otherwise asks nuget.org whether a newer vpk exists on every
|
||||
# run, which is a network call whose answer this build must not act on: the version that
|
||||
# packs is the one in the tool manifest.
|
||||
#
|
||||
# No `vpk download`, where the release script has one. That fetches the previous release so
|
||||
# deltas can be built against it, which needs the feed and produces an artefact nobody
|
||||
# applies. A full package proves the packaging.
|
||||
dotnet vpk '[win]' pack \
|
||||
--skip-updates \
|
||||
--packId DodoSSH.Desktop \
|
||||
--packVersion "$packVersion" \
|
||||
--packDir "$RUNNER_TEMP/win-x64" \
|
||||
--packTitle DodoSSH \
|
||||
--packAuthors DodoTech \
|
||||
--mainExe DodoSSH.exe \
|
||||
--icon src/DodoSSH.Client.App/Assets/dodossh.ico \
|
||||
--runtime win-x64 \
|
||||
--channel win \
|
||||
--outputDir "$releases"
|
||||
|
||||
# Asked for rather than assumed. vpk fails loudly and this costs a line, but the artefact
|
||||
# whose existence is the entire question — a Windows setup stub built on a machine that is
|
||||
# not Windows — is worth naming rather than inferring from an exit code.
|
||||
setup="$releases/DodoSSH.Desktop-win-Setup.exe"
|
||||
if [ ! -s "$setup" ]; then
|
||||
echo "vpk reported success and there is no setup executable at $setup." >&2
|
||||
ls -la "$releases" >&2 || true
|
||||
exit 1
|
||||
fi
|
||||
|
||||
ls -la "$releases"
|
||||
echo "Packaged DodoSSH $packVersion for win-x64, from a build MinVer calls $version."
|
||||
|
||||
# This includes the end-to-end suite, which starts PostgreSQL, Keycloak and an OpenSSH
|
||||
# server through Testcontainers and runs the API as a child process — so it needs a
|
||||
@@ -932,18 +1067,27 @@ jobs:
|
||||
if: always() && github.event_name != 'pull_request'
|
||||
run: docker logout registry-docker.dodotech.cloud
|
||||
|
||||
# There is no job here that publishes the desktop client, and there is not going to be one. Two
|
||||
# independent reasons, and both need saying because someone will fix one and think they are done.
|
||||
# There is no job here that publishes the desktop client, and there is not going to be one. It is built
|
||||
# and packaged here — the two steps at the end of the build job — and what is withheld is only the
|
||||
# upload.
|
||||
#
|
||||
# The smaller one is mechanical: vpk stamps and embeds the Setup.exe and Update.exe stubs with Windows
|
||||
# tooling, and every job in this file is runs-on: [linux]. A Windows runner would answer that.
|
||||
# ◆ ONE REASON, WHERE THIS ONCE CLAIMED TWO, AND THE SECOND WAS NOT TRUE.
|
||||
#
|
||||
# The larger one is that a Windows runner would not answer the other. Velopack clients fetch from the
|
||||
# release feed and do not verify a package signature when they apply it, so whoever can write a release
|
||||
# on this repository can publish an update that every installed client downloads and runs. That is the
|
||||
# same capability as the signing key, reached through a different door — and docs/adr/0011 rule 1 puts
|
||||
# that capability on a machine which is not a runner, because a workflow secret is held by everyone who
|
||||
# can change a workflow file. See docs/adr/0013-desktop-distribution-and-updates.md.
|
||||
# It used to say that vpk stamps and embeds the Setup.exe and Update.exe stubs with Windows tooling and
|
||||
# so could not run on a Linux runner, and offered that as the smaller of two reasons. It was wrong, and
|
||||
# worth recording as wrong because it is the kind of claim that discourages anybody from trying: vpk
|
||||
# cross-compiles when told to, `vpk [win] pack` on this runner builds the setup stub, the portable zip
|
||||
# and the .nupkg, and only signing needs Windows — which this repository does not do yet. That is what
|
||||
# the packaging step now does on every main and tag build.
|
||||
#
|
||||
# The reason that stands is the one that was never mechanical. Velopack clients fetch from the release
|
||||
# feed and do not verify a package signature when they apply it, so whoever can write a release on this
|
||||
# repository can publish an update that every installed client downloads and runs. That is the same
|
||||
# capability as the signing key, reached through a different door — and docs/adr/0011 rule 1 puts that
|
||||
# capability on a machine which is not a runner, because a workflow secret is held by everyone who can
|
||||
# change a workflow file. A Windows runner would never have answered it, which is why fixing the
|
||||
# mechanical half changes nothing about the division of labour.
|
||||
# See docs/adr/0013-desktop-distribution-and-updates.md.
|
||||
#
|
||||
# What cuts a release is scripts/release-windows.ps1, run by a person. What this file does is prove the
|
||||
# thing still builds and packages, which is the same division of labour the android job above already
|
||||
|
||||
@@ -85,13 +85,38 @@ packs and stops. Phase two, a separate invocation, asks for the token and upload
|
||||
what reaches users has been installed and started by a human first, and so that the credential is in memory
|
||||
only for the minutes that need it.
|
||||
|
||||
There is a second, smaller reason the release could not be a CI job here anyway: `vpk` stamps and embeds
|
||||
the `Setup.exe` and `Update.exe` stubs with Windows tooling, and every job in `ci.yml` is
|
||||
`runs-on: [linux]`. Both reasons are recorded because somebody will fix one and believe they are done.
|
||||
A second, smaller reason was recorded here — that `vpk` stamps and embeds the `Setup.exe` and
|
||||
`Update.exe` stubs with Windows tooling, and every job in `ci.yml` is `runs-on: [linux]` — on the
|
||||
reasoning that somebody will fix one reason and believe they are done.
|
||||
|
||||
What CI does gain is the same thing the `android` job already does — it proves the artefact still builds.
|
||||
A `win-x64` publish runs on main and on tags, so a restore graph that resolves for `net10.0` and not for
|
||||
`net10.0/win-x64` fails there rather than under a person midway through a release.
|
||||
◆ **That second reason was not true, and the correction is worth more than the claim was.** `vpk`
|
||||
cross-compiles when it is told to: the bracketed OS directive, `vpk [win] pack`, builds the setup stub,
|
||||
the portable bundle and the `.nupkg` on Linux, and reports `Directive enabled for cross-compiling from
|
||||
Linux (current os) to Windows` while doing it. Only signing needs Windows tooling, and rule 8 above is
|
||||
why nothing is signed yet. Plain `vpk pack` on Linux targets Linux and rejects a Windows RID, which is
|
||||
the shape of the mistake: the tool does not fail because it cannot, it fails because it was not asked.
|
||||
|
||||
The correction does not move this rule at all, and that is the point of separating the two. The reason
|
||||
that stands was never mechanical, so a Linux runner that *can* pack changes nothing: what may not be on
|
||||
a runner is the token, not the build.
|
||||
|
||||
What CI gains is the same thing the `android` job already does — it proves the artefact still builds and
|
||||
still packages. On main and on tags the build job publishes `win-x64` and then packs it, so a restore
|
||||
graph that resolves for `net10.0` and not for `net10.0/win-x64` fails there rather than under a person
|
||||
midway through a release. Packaging is the more valuable half and adds a few seconds to a publish that
|
||||
has already happened — measured at two to seven, against the minutes the publish itself takes: `vpk` verifies
|
||||
that the main executable actually calls `VelopackApp.Build().Run()`, so the change that quietly produces
|
||||
an application which never updates itself — compiling, tested, and invisible until a release goes
|
||||
unreceived — fails in CI. The packages are written to `RUNNER_TEMP` and discarded, and are deliberately
|
||||
not offered as workflow artefacts: an installer nobody has run should not sit somewhere that invites
|
||||
passing it on.
|
||||
|
||||
CI does not call `scripts/release-windows.ps1` to do any of this, and should not. That script is the
|
||||
person's procedure — it refuses a dirty tree, requires a tagged `HEAD`, fetches the previous release for
|
||||
deltas, and prompts for the token — so running it on a runner would mean either weakening it with CI
|
||||
switches or having CI satisfy conditions that only make sense at a desk. The workflow writes the same
|
||||
`vpk` arguments out in shell, and the constants they share (pack id, title, authors, channel, icon) are
|
||||
noted in both places as a contract with `VelopackUpdateChannel` and with every installed client.
|
||||
|
||||
### 4. The update check points at the project's forge, and the address is a constant
|
||||
|
||||
|
||||
+27
-2
@@ -568,12 +568,37 @@ Packaging the desktop client had broken the server's image build, and nothing bu
|
||||
have caught it. Found by running `docker build` locally rather than by reading the lock files.
|
||||
|
||||
So the RID stays out of the committed state, and the two commands that need one — the release script's
|
||||
publish and the `windows publish still resolves` step in `ci.yml` — pass `-p:RestoreLockedMode=false` for
|
||||
themselves alone. That restore rewrites the lock files as a side effect, which does not matter on a runner
|
||||
publish and the `publish the windows desktop client` step in `ci.yml` — pass `-p:RestoreLockedMode=false`
|
||||
for themselves alone. That restore rewrites the lock files as a side effect, which does not matter on a runner
|
||||
whose checkout is discarded and does matter on a developer's machine, so the release script runs
|
||||
`git checkout -- '*packages.lock.json'` afterwards. `-p:RestorePackagesWithLockFile=false` is not an
|
||||
alternative: it fails NU1005 whenever a lock file already exists.
|
||||
|
||||
**`vpk` picks its target from the host, and cross-compiling is a bracketed directive rather than a flag.**
|
||||
The CI job packages the Windows desktop client on a Linux runner, and plain `vpk pack --runtime win-x64`
|
||||
there refuses outright:
|
||||
|
||||
```
|
||||
To build packages for Linux, the target rid must be Linux (actually was Windows). If your real intention
|
||||
was to cross-compile a release for Windows then you should provide an OS directive: eg. 'vpk [win] pack ...'
|
||||
```
|
||||
|
||||
The directive goes before the verb — `dotnet vpk '[win]' pack …` — and must be quoted in a POSIX shell,
|
||||
where `[win]` is a glob matching any one of `w`, `i` and `n`. With it, a Linux runner logs
|
||||
`Directive enabled for cross-compiling from Linux (current os) to Windows` and writes
|
||||
`DodoSSH.Desktop-win-Setup.exe`, the portable zip, the `.nupkg` and `releases.win.json` — the same set a
|
||||
Windows machine produces. `--runtime win-x64` is still required: the directive says which OS is being
|
||||
targeted, not which RID. Only signing needs Windows tooling, which is why `ci.yml` can package and
|
||||
`scripts/release-windows.ps1` will still be the thing that signs when there is a certificate to sign with.
|
||||
|
||||
**`dotnet msbuild -getProperty:Version` answers `1.0.0`, and MinVer is not to blame.** `-getProperty`
|
||||
without a target *evaluates* the project and runs nothing, while MinVer computes the version inside a
|
||||
target — so the read comes back as the SDK default on a full checkout with every tag present, which looks
|
||||
exactly like a version that was never configured. `-t:MinVer` makes `-getProperty` report the value after
|
||||
that target has run, and both readers of it — the tag check in `ci.yml` and `scripts/release-windows.ps1`
|
||||
— pass it. Neither had, and neither had ever run: the CI check is `if:` a tag ref and there are no tags
|
||||
yet, so the first release would have been refused by its own guard, which would have blamed `fetch-depth`.
|
||||
|
||||
**The Android head's lock file is outside the solution, so nothing checks it until the android job runs
|
||||
— and the android job was broken for an unrelated reason for the whole of the release that went stale.**
|
||||
`DodoSSH.Client.Android` is deliberately not in `DodoSSH.slnx` (it needs a workload the other two jobs
|
||||
|
||||
@@ -100,7 +100,12 @@ Push-Location $RepoRoot
|
||||
try {
|
||||
# ---- What is being released -----------------------------------------------------------------------
|
||||
|
||||
$version = (& dotnet msbuild $Project -getProperty:Version -nologo) -replace '\s', ''
|
||||
# -t:MinVer, and it is load-bearing. -getProperty on its own evaluates the project and runs no
|
||||
# targets, while MinVer sets Version from inside one — so this read answered the SDK's default
|
||||
# 1.0.0 regardless of the tag, and the check below would then have refused to build anything not
|
||||
# tagged v1.0.0. CI's tag check had the same line and the same fault; both are fixed, and both
|
||||
# say so, because this is the version that ends up in the package a client compares against.
|
||||
$version = (& dotnet msbuild $Project -getProperty:Version -t:MinVer -nologo) -replace '\s', ''
|
||||
if ([string]::IsNullOrWhiteSpace($version)) {
|
||||
Stop-With 'Could not read the version from MSBuild.'
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user