Use mapped version for update notification check

[pljones]

Short description of changes

When the Connect Dialog was updated to sort on server version number, we had to add a clever algorithm to allow it to get the version numbers in "Jamulus Version" order, which is somewhat non-standard.

Previously, there was an explicit guard in the update check to avoid a local version with a non-standard version running the version check, because the version check logic used couldn't handle it.

So this change:

• puts the new custom version comparator into common code
• updates the Connect Dialog to use it
• updates Client and Server version checks to use it
• removes the guard in Client and Server version checks

CHANGELOG: Use mapped version for update notification check

Context: Fixes an issue?

Fixes: #3669

Demo:

Current "latest" is release 3.12.3. With our current versioning scheme, that means main and release/3_12 have 3.12.3dev as the version, going to 4.0.0 or 3.12.4 as appropriate. Say we had cut a pre-release for 3.12.3, though - 3.12.3beta1:

$ ./Jamulus --version
 *** Jamulus, Version 3.12.3beta1

Given 3.12.3 is on the update servers, I'd expect my old beta to get a notification...

No update notification.

Does this change need documentation? What needs to be documented and how?

No.

Status of this Pull Request

Tested as follows:

1. Install a release version of the client or server with a version number less than current, run the GUI

(Test omitted -- this isn't being applied to old builds, only future builds. Examples below use old build numbers just to use existing update check servers.)

1. Prove no regression

   3. Install a development version of the next release (3.x.xdev...), run the GUI

   • Set ChangeLog and Jamulus.pro to 3.12.1dev (i.e. prior to current release)
   • Run GUI Client with --showallservers
     • Verify Update Notification shown (as current)
     • Verify Connect Dialog sort order for server version is correct
   • Run GUI Server
     • Verify Update Notification shown (as current)

   Client

   

   Server

   

2. Prove fix works

   5. Install a tagged pre-release version of the next release (tag in "alpha*", "beta*" and "rc*"), run the GUI

   • Set ChangeLog and Jamulus.pro to 3.12.1alpha (i.e. prior to current release but )
   • Run GUI Client with --showallservers
     • Verify Update Notification shown (as current)
     • Verify Connect Dialog sort order for server version is correct
   • Run GUI Server
     • Verify Update Notification shown (as current)

   Client
   

   

   Server

What is missing until this pull request can be merged?

There's a couple of tests I skipped but I think it's pretty sound.

Checklist

• I've verified that this Pull Request follows the general code principles
• I tested my code and it does what I want
• My code follows the style guide
• I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
• I've filled all the content above

[Copilot]

Pull request overview

This PR centralizes Jamulus-specific version-string comparison logic and uses it for both the Connect Dialog version sorting and the client/server “update available” notification heuristic, so non-release builds (e.g., rc/beta) are compared in a consistent “Jamulus Version” order.

Changes:

• Adds a shared CompareVersionStrings() helper in util for comparing Jamulus version strings.
• Updates Connect Dialog server-version sorting to use the shared comparator instead of an in-file mapping helper.
• Updates Client/Server dialogs’ update notification check to use the shared comparator (removing the previous suffix-guarded QVersionNumber comparison).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/util.h	Declares the new shared version comparison helper.
src/util.cpp	Implements version mapping + CompareVersionStrings() used by UI sorting and update checks.
src/connectdlg.cpp	Switches server version sorting to use the shared comparator and stores raw version strings.
src/clientdlg.cpp	Uses the shared comparator for update notification check.
src/serverdlg.cpp	Uses the shared comparator for update notification check.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[mcfnord]

MY LLM WROTE:

Removing the strVersion.size() == serverSuffixIndex guard changes behaviour for version strings that don't parse. Those hit MapVersionStrForCompare's return versionStr fallback and get compared as raw text against a zero-padded key, so anything starting above '0' sorts as newer and shows the update notification.

Extracted the comparator verbatim, built against Qt 5.15.13, mine = 3.12.3:

remote	before	after
unknown	hide	show
abc	hide	show
3.12	hide	show
v3.12.3	hide	show
`` (empty)	hide	hide

The old guard rejected all of these. Might be worth having the fallback return "not newer" rather than a comparable string, since it's a remote-supplied value.

[pljones]

Ah, yes, I tried to get GPT to explain the removal. It should block remote servers with dubious versions for the version checks.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/util.cpp:92

• MapVersionStrForCompare() appends the raw pre-release suffix, so lexicographic sorting will order ...beta10 before ...beta2. Since this mapped key is now used for Connect dialog sorting (and could affect any future comparisons), it should normalize/pad the numeric part for alpha|beta|rc suffixes.

    if ( suffix.isEmpty() )
    {
        x = "="; // bare version number
    }
    else if ( suffix.startsWith ( "rc" ) || suffix.startsWith ( "beta" ) || suffix.startsWith ( "alpha" ) )
    {
        x = "<"; // pre-release version
    }

src/util.cpp:55

• GetSemVerRegex() currently matches strings like 3.12.3- or 3.12.3: (because -? and :?(.*) allow empty captures). That makes IsReleaseVersion() treat those non-bare versions as releases, contrary to the intent of only accepting strict x.y.z as a release version, and also makes MapVersionStrForCompare() map them as "bare" (=). Tighten the regex so a trailing -/: without content does not match.

const QRegularExpression& GetSemVerRegex()
{
    static const QRegularExpression semVerRegex ( R"(^(\d+)\.(\d+)\.(\d+)-?(.*):?(.*)$)" );
    return semVerRegex;

src/util.cpp:60

• The comment above IsReleaseVersion() says "Only bare x.y.z versions should trigger update notifications", but this helper is used to filter remote versions (from the directory/server) before comparing against the local VERSION. Clarifying this avoids future confusion about whether non-release local builds should or shouldn't be considered.

// Only bare x.y.z versions should trigger update notifications.
bool IsReleaseVersion ( const QString& version )
{

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/util.cpp:56

• The comment describing the regex capture layout is inconsistent: the release example omits one of the optional capture fields (4/5/6). This can mislead future changes to the comparator regex.

// Examples:
//   3.10.1 -> (3,10,1,"","")
//   3.10.1dev-2f2d2a1:1756656363 -> (3,10,1,"dev-2f2d2a1","1756656363","")