build: preserve semantic version separators

[pucedoteth]

btcd hit this in its own copy of version.go and fixed it in 65db493 ("version: preserve semantic version separators", btcsuite/btcd#2578): SemVer separates pre-release identifiers and build metadata with periods, but semanticAlphabet omitted the period, so normalizeVerString silently collapsed beta.rc1 into betarc1.

btcwallet/build/version.go carries the same code and still has the old alphabet:

btcd/version.go:14           ...uvwxyz-."
btcwallet/build/version.go:14 ...uvwxyz-"

btcd's test, run unchanged against this copy, fails:

--- FAIL: TestNormalizeVerString
    version_test.go:25: unexpected normalized version: got "betarc1", want "beta.rc1"

This is reachable rather than latent. normalizeVerString is applied to appBuild, which the comment above it documents as settable at build time:

// appBuild is defined as a variable so it can be overridden during the build
// process with '-ldflags "-X main.appBuild foo' if needed.

SemVer build metadata is dot-separated — exp.sha.5114f85 is an example from the spec itself — so a btcwallet built with that stamp reports 0.15.1-alpha+expsha5114f85, with the separators dropped and the metadata no longer round-tripping.

The change is the one-character fix from 65db493 plus that commit's test, with a build metadata case added for the appBuild path.

go build ./..., go test ./build/, gofmt and go vet are clean.

Two things I noticed but left alone to keep the diff to the fix:

• That doc comment says -X main.appBuild, but this copy lives in package build, so the flag is really -X github.com/btcsuite/btcwallet/build.appBuild. It was accurate before the code moved out of main.
• btcctl: preserve semantic version separators btcd#2596 applies the same fix to btcd/cmd/btcctl/version.go, which is a third copy of this file that was also missed.

---

Disclosure: written with AI assistance; the analysis, fix and test were verified locally.

[Lrifton92]

Correct fix, and there is a stronger argument for it than the PR description makes: btcwallet is the odd one out in its own family. Both sibling projects already allow the dot —

• btcsuite/btcd version.go:14
• lightningnetwork/lnd build/version.go:38

both use "0123456789...xyz-.", identical to what this PR lands. So this is not a behaviour change so much as btcwallet catching up, which also means -X build.appBuild=exp.sha.5114f85 currently produces a different version string in btcwallet than the exact same flag does in btcd. Worth putting in the commit message.

The test cannot fail if the filter is deleted

tests := []string{"beta.rc1", "exp.sha.5114f85"}
for _, version := range tests {
    if got := normalizeVerString(version); got != version {

Every case asserts normalizeVerString(x) == x, which is equally true of an identity function. Removing the strings.ContainsRune check entirely — or dropping the whole loop and returning str — passes this test unchanged. It pins the new half of the behaviour and leaves the old half unguarded.

One invalid-character case fixes that and costs a line, something like {in: "beta!rc1", want: "betarc1"} alongside the pass-through ones. Then the test distinguishes "dots survive" from "nothing is filtered", which is the actual claim.

Minor, and inherited rather than introduced

Allowing . in a character filter also lets malformed input through: appBuild = "beta." now yields 0.15.1+beta., and ".." yields 0.15.1+.., neither of which is a valid semver string — where previously both were stripped to empty and the suffix was dropped. SemVer requires dot-separated identifiers that are each non-empty, and a per-character allowlist cannot express that.

I would not hold the PR for it, because btcd and lnd have exactly the same naive filter with no further validation, so tightening here would make btcwallet inconsistent again in the other direction. But since appBuild is set from build scripts via -ldflags, a typo now propagates into Version() instead of being neutralised. If you want to acknowledge it cheaply, the comments on appPreRelease and appBuild currently say the value "MUST only contain characters from semanticAlphabet per the semantic versioning spec" — that is now necessary but no longer sufficient, and saying so would save the next reader the same detour.

[pucedoteth]

All three addressed — thanks, the second one was a real hole.

Test could not fail if the filter were removed. Correct, and I verified it: every case I wrote asserted normalizeVerString(x) == x, which an identity function satisfies. The test is now table-driven with your beta!rc1 → betarc1 case alongside the pass-through ones.

I mutation-tested the result rather than eyeballing it. Replacing the body with return str only proves a build failure (unused imports), so I neutralised the check in place instead — if true || strings.ContainsRune(...) — which still compiles:

--- FAIL: TestNormalizeVerString
    version_test.go:42: character outside the alphabet: normalizeVerString("beta!rc1") = "beta!rc1", want "betarc1"

Restored, it passes. Both halves are now pinned.

btcwallet is the family outlier. Confirmed and moved into the commit message: btcd/version.go:14 and lnd/build/version.go:38 are both already "…xyz-.", so the same -ldflags stamp yields a different version string in btcwallet than in btcd today. That framing is better than mine — it is catching up, not changing behaviour.

Per-character filter cannot express dot-separated identifiers. Also confirmed:

normalizeVerString("beta.") = "beta."
normalizeVerString("..")    = ".."

Both previously stripped to empty and dropped the suffix. I took the cheap option you suggested and left the validation alone, since btcd and lnd carry the identical naive filter and tightening only here would re-introduce the divergence in the other direction. The comments on appPreRelease and appBuild now say the alphabet is necessary but not sufficient, and that the value must also be a well-formed dot-separated list of non-empty identifiers.

go build ./..., go test ./build/, gofmt and go vet are clean.