[GH-ISSUE #388] Is that version comparison implementation correct? #1156

New Issue

GiteaMirror · 2026-04-16T10:25:40-05:00

GiteaMirror commented

2026-04-16 10:25:40 -05:00

Originally created by @handicraftsman on GitHub (Aug 22, 2017).
Original GitHub issue: https://github.com/semver/semver/issues/388

SemverVersionDifference semver_version_compare(SemverVersion ver1, SemverVersion ver2) {
  if (ver1.major > ver2.major)
    return SemverVersionDifference_GREATER;
  if (ver1.major < ver2.major)
    return SemverVersionDifference_LESS;

  if (ver1.minor > ver2.minor)
    return SemverVersionDifference_GREATER;
  if (ver1.minor < ver2.minor)
    return SemverVersionDifference_LESS;

  if (ver1.patch > ver2.patch)
    return SemverVersionDifference_GREATER;
  if (ver1.patch < ver2.patch)
    return SemverVersionDifference_LESS;
  if (ver1.patch == ver2.patch && !ver1.is_prerelease && !ver2.is_prerelease)
    return SemverVersionDifference_EQUAL;

  if (!ver1.is_prerelease && ver2.is_prerelease)
    return SemverVersionDifference_GREATER;
  if (ver1.is_prerelease && !ver2.is_prerelease)
    return SemverVersionDifference_LESS;

  unsigned int l1 = strlen(ver1.prerelease_string);
  unsigned int l2 = strlen(ver2.prerelease_string);
  for (int i = 0; i < l1; i++) {
    if (i == l1-1)
      if (l2 > l1)
        return SemverVersionDifference_LESS;
    if (i == l2-1)
      if (l1 > l2)
        return SemverVersionDifference_GREATER;

    if (isdigit(ver1.prerelease_string[i]) && !isdigit(ver2.prerelease_string[i]))
      return SemverVersionDifference_LESS;
    if (!isdigit(ver1.prerelease_string[i]) && isdigit(ver2.prerelease_string[i]))
      return SemverVersionDifference_GREATER;

    if (ver1.prerelease_string[i] < ver2.prerelease_string[i])
      return SemverVersionDifference_LESS;
    if (ver1.prerelease_string[i] > ver2.prerelease_string[i])
      return SemverVersionDifference_GREATER;
  }

  return SemverVersionDifference_EQUAL;
}

Originally created by @handicraftsman on GitHub (Aug 22, 2017). Original GitHub issue: https://github.com/semver/semver/issues/388 ```c SemverVersionDifference semver_version_compare(SemverVersion ver1, SemverVersion ver2) { if (ver1.major > ver2.major) return SemverVersionDifference_GREATER; if (ver1.major < ver2.major) return SemverVersionDifference_LESS; if (ver1.minor > ver2.minor) return SemverVersionDifference_GREATER; if (ver1.minor < ver2.minor) return SemverVersionDifference_LESS; if (ver1.patch > ver2.patch) return SemverVersionDifference_GREATER; if (ver1.patch < ver2.patch) return SemverVersionDifference_LESS; if (ver1.patch == ver2.patch && !ver1.is_prerelease && !ver2.is_prerelease) return SemverVersionDifference_EQUAL; if (!ver1.is_prerelease && ver2.is_prerelease) return SemverVersionDifference_GREATER; if (ver1.is_prerelease && !ver2.is_prerelease) return SemverVersionDifference_LESS; unsigned int l1 = strlen(ver1.prerelease_string); unsigned int l2 = strlen(ver2.prerelease_string); for (int i = 0; i < l1; i++) { if (i == l1-1) if (l2 > l1) return SemverVersionDifference_LESS; if (i == l2-1) if (l1 > l2) return SemverVersionDifference_GREATER; if (isdigit(ver1.prerelease_string[i]) && !isdigit(ver2.prerelease_string[i])) return SemverVersionDifference_LESS; if (!isdigit(ver1.prerelease_string[i]) && isdigit(ver2.prerelease_string[i])) return SemverVersionDifference_GREATER; if (ver1.prerelease_string[i] < ver2.prerelease_string[i]) return SemverVersionDifference_LESS; if (ver1.prerelease_string[i] > ver2.prerelease_string[i]) return SemverVersionDifference_GREATER; } return SemverVersionDifference_EQUAL; } ```

GiteaMirror closed this issue

2026-04-16 10:25:41 -05:00

GiteaMirror commented

2026-04-16 10:25:42 -05:00

@handicraftsman commented on GitHub (Aug 22, 2017):

Typedefs:

typedef enum SemverVersionDifference {
  SemverVersionDifference_EQUAL,
  SemverVersionDifference_LESS,
  SemverVersionDifference_GREATER
} SemverVersionDifference;

typedef struct SemverVersion {
  unsigned int major;
  unsigned int minor;
  unsigned int patch;
  bool is_prerelease;
  char* prerelease_string;
} SemverVersion;

@handicraftsman commented on GitHub (Aug 22, 2017): Typedefs: ```c typedef enum SemverVersionDifference { SemverVersionDifference_EQUAL, SemverVersionDifference_LESS, SemverVersionDifference_GREATER } SemverVersionDifference; typedef struct SemverVersion { unsigned int major; unsigned int minor; unsigned int patch; bool is_prerelease; char* prerelease_string; } SemverVersion; ```

GiteaMirror commented

2026-04-16 10:25:43 -05:00

@FichteFoll commented on GitHub (Aug 22, 2017):

How about using unittests?

I used this earlier, though they are based on semver 2.0.0-rc.2 and therefore include ordering of metadata as well, but generally this is how you could do it.

a5f78ecc46/_test.py (L32-L50)

(actual tests in line 70)

@FichteFoll commented on GitHub (Aug 22, 2017): How about using unittests? I used this earlier, though they are based on semver 2.0.0-rc.2 and therefore include ordering of metadata as well, but generally this is how you could do it. https://github.com/FichteFoll/pysemver/blob/a5f78ecc461ac638d86ac51ac31387f38523278c/_test.py#L32-L50 (actual tests in line 70)

GiteaMirror commented

2026-04-16 10:25:46 -05:00

@handicraftsman commented on GitHub (Aug 23, 2017):

I am not actually familiar with unittests in C. So i am just using a test program. But i should really move to unittests for such stuff after finishing the whole semver implementation.

@handicraftsman commented on GitHub (Aug 23, 2017): I am not actually familiar with unittests in C. So i am just using a test program. But i should really move to unittests for such stuff after finishing the whole semver implementation.

GiteaMirror commented

2026-04-16 10:25:47 -05:00

@jwdonahue commented on GitHub (Oct 11, 2018):

@handicraftsman said:

But i should really move to unittests for such stuff after finishing the whole semver implementation.

Many would argue that you should never have started without them.

The answer to your question is no, your logic is broken. You are trying to process the prerelease field as if it were a simple string of characters. That is incorrect. From the spec #11:

Precedence for two pre-release versions with the same major, minor, and patch version MUST be determined by comparing each dot separated identifier from left to right until a difference is found as follows: identifiers consisting of only digits are compared numerically and identifiers with letters or hyphens are compared lexically in ASCII sort order. Numeric identifiers always have lower precedence than non-numeric identifiers. A larger set of pre-release fields has a higher precedence than a smaller set, if all of the preceding identifiers are equal.

As you can see, you must identify the type of each identifier first, then apply the appropriate logic.

@jwdonahue commented on GitHub (Oct 11, 2018): @handicraftsman said: > But i should really move to unittests for such stuff after finishing the whole semver implementation. Many would argue that you should never have started without them. The answer to your question is no, your logic is broken. You are trying to process the prerelease field as if it were a simple string of characters. That is incorrect. From the [spec #11](https://semver.org/#spec-item-11): > Precedence for two pre-release versions with the same major, minor, and patch version MUST be determined by comparing each dot separated identifier from left to right until a difference is found as follows: identifiers consisting of only digits are compared numerically and identifiers with letters or hyphens are compared lexically in ASCII sort order. Numeric identifiers always have lower precedence than non-numeric identifiers. A larger set of pre-release fields has a higher precedence than a smaller set, if all of the preceding identifiers are equal. As you can see, you must identify the type of each identifier first, then apply the appropriate logic.

GiteaMirror commented

2026-04-16 10:25:49 -05:00

@jwdonahue commented on GitHub (Oct 11, 2018):

@handicraftsman , please take any further code reviews to Stackoverflow and use the semver tag and please close this issue at your earliest possible convenience.

@jwdonahue commented on GitHub (Oct 11, 2018): @handicraftsman , please take any further code reviews to Stackoverflow and use the semver tag and please close this issue at your earliest possible convenience.

GiteaMirror commented

2026-04-16 10:25:51 -05:00

@handicraftsman commented on GitHub (Oct 11, 2018):

Okay, sorry. Completely forgot about this issue ;shrugs;

@handicraftsman commented on GitHub (Oct 11, 2018): Okay, sorry. Completely forgot about this issue ;shrugs;

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/semver#1156