fix(version): ignore SemVer build metadata and compare long numbers

skills/dload-fetch-tool/SKILL.md

@@ -45,7 +45,7 @@ Done when the block has a `<repository>` whose `asset-pattern` matches every OS/
 | Attribute | Purpose |
 |---|---|
 | `software` | Alias or name of the tool (built-in or inline). Required. |
-| `version` | Composer-style constraint: `^2025.1`, `~1.0.0`, `^2.12.0@beta`, `^2.12.0-hotfix@alpha` (feature builds, alpha ones included), one pre-release like `2025.1.0-rc.2`, or a range from one like `^2025.1.0-rc.1`. Omit for latest stable. Pre-releases order by stability (`nightly < snapshot < dev < unstable < alpha < preview < beta < pre < RC < stable`), then by number; `stable` by default. A pre-release in a constraint sets the minimum stability for any operator, even `<3.5.0-beta.2`; lower it with `@alpha`/`@dev`. A keyword without a number is a bound too: `>2.0-beta` takes `2.0.0-RC1` and `2.0.0`. An exact version with a pre-release or another suffix is that one tag: `2.0-beta` skips `2.0.0-beta.3`, `2.12.0-hotfix` skips `2.12.0-hotfix.2`; after `^`, `~` or a comparison a non-stability suffix is a feature the tag must contain. A tagged build like `2025.1.0-rc.2-linux` orders right after its pre-release: an exact `2025.1.0-rc.2` skips it, `>2025.1.0-rc.2` takes it. Numbers may have more than three parts (`1.2.3.4`), in constraints too (`>=1.2.3.4.5`); trailing zero parts don't count (`1.2.3` = `1.2.3.0`). Case doesn't matter (`-ARM64` = `-arm64`, `^1.0-Linux` takes `1.0.0-linux`), and a pre-release may be glued to the number as in tags (`^2.0.0rc1`). |
+| `version` | Composer-style constraint: `^2025.1`, `~1.0.0`, `^2.12.0@beta`, `^2.12.0-hotfix@alpha` (feature builds, alpha ones included), one pre-release like `2025.1.0-rc.2`, or a range from one like `^2025.1.0-rc.1`. Omit for latest stable. Pre-releases order by stability (`nightly < snapshot < dev < unstable < alpha < preview < beta < pre < RC < stable`), then by number; `stable` by default. A pre-release in a constraint sets the minimum stability for any operator, even `<3.5.0-beta.2`; lower it with `@alpha`/`@dev`. A keyword without a number is a bound too: `>2.0-beta` takes `2.0.0-RC1` and `2.0.0`. An exact version with a pre-release or another suffix is that one tag: `2.0-beta` skips `2.0.0-beta.3`, `2.12.0-hotfix` skips `2.12.0-hotfix.2`; after `^`, `~` or a comparison a non-stability suffix is a feature the tag must contain. A tagged build like `2025.1.0-rc.2-linux` orders right after its pre-release: an exact `2025.1.0-rc.2` skips it, `>2025.1.0-rc.2` takes it. Numbers may have more than three parts (`1.2.3.4`), in constraints too (`>=1.2.3.4.5`); trailing zero parts and build metadata don't count (`1.2.3` = `1.2.3.0` = `1.2.3+build.5`). Case doesn't matter (`-ARM64` = `-arm64`, `^1.0-Linux` takes `1.0.0-linux`), and a pre-release may be glued to the number as in tags (`^2.0.0rc1`). |
 | `extract-path` | Target folder (default: project root). |
 | `type` | `binary` (default), `phar` (required for PHAR — skips extraction), `archive` (unpack the whole asset keeping its folder layout). |
 

src/Module/Binary/Binary.php

@@ -27,7 +27,7 @@ public function getPath(): Path;
     public function exists(): bool;
 
     /**
-     * Gets the binary version.
+     * Gets the binary version as printed, with its pre-release and build metadata, e.g. `2.3.4+20230415`.
      *
      * @return non-empty-string|null Version string or null if not available
      */

src/Module/Binary/BinaryVersion.php

@@ -24,7 +24,7 @@ public static function fromBinaryOutput(string $output): static
     {
         // Try to extract version using semantic version pattern
         $version = \preg_match(self::outputVersionPattern(), $output, $matches)
-            ? $matches[1] . ($matches[2] ?? '')
+            ? $matches[1] . ($matches[2] ?? '') . ($matches[3] ?? '')
             : self::extractVersionWithFallbacks($output);
 
         if ($version === null) {
@@ -45,7 +45,8 @@ public static function fromBinaryOutput(string $output): static
     private static function outputVersionPattern(): string
     {
         return '/(?:version:?\s*)?(?:v(?:er(?:sion)?)?\.?\s*)?' . parent::VERSION_SEMVER_NUMBER_PATTERN
-            . '((?:[-+]|(?=(?![ab](?![a-z]|[._-]?\d))' . PreRelease::wordPattern() . '))[\w.-]+)?/i';
+            . '((?:[-+]|(?=(?![ab](?![a-z]|[._-]?\d))' . PreRelease::wordPattern() . '))[\w.-]+)?'
+            . '(' . parent::buildPattern() . ')?/i';
     }
 
     /**

src/Module/Binary/Internal/AbstractBinary.php

@@ -89,7 +89,9 @@ public function getVersion(): ?BinaryVersion
 
     public function getVersionString(): ?string
     {
-        return $this->getVersion()?->number;
+        // The whole version as printed, with its pre-release and build metadata
+        $version = $this->getVersion()?->string;
+        return $version === '' ? null : $version;
     }
 
     abstract public function getPath(): Path;

src/Module/Version/Constraint.php

@@ -16,6 +16,7 @@
  * - Combined constraints: ^2.12.0-feature@beta
  * - Exact releases, a pre-release or a tail included: 3.5.0-beta.1, 2.0-beta, 1.0.0-1, 2.12.0-hotfix, 2.0.0rc1
  * - Pre-release bounds of ranges and comparisons: >=3.5.0-beta.1 <3.5.0-RC1, ^1.0 || ^2.0-beta.1, >2.0-beta, ^2.0.0rc1
+ * - Build metadata, which does not count: >=1.2.3+build.5, 3.5.0-beta.1+5
  *
  * A stability keyword at the start or the end of a feature suffix sets the minimum stability
  * unless an explicit one is given: ^2.12.0-my-feature-beta.
@@ -68,6 +69,9 @@ private function __construct(
             );
         }
 
+        # Build metadata does not count, and a hyphen within it, as in "1.2.3+build-5", starts no feature suffix
+        $origin = (string) \preg_replace('/(?<=[a-z\d])' . Version::buildPattern() . '(?=$|[\s,|])/i', '', $origin);
+
         # A single version with a pre-release or any other tail names one release: "2.0-beta" accepts neither
         # "2.0.0-beta.3" nor "2.0.0", and "2.12.0-hotfix" not "2.12.0-hotfix.2"; a bare number covers every build
         # of that number, see Comparison. In a range, a pre-release bound, a bare "-beta" included, is a part of
@@ -77,7 +81,7 @@ private function __construct(
         $tail = '';
         $preRelease = null;
         if (\preg_match(
-            '/^(?:==?)?\s*v?(\d+(?:\.\d+)*(?:\+\d+)?)(-[\w.-]+|[._]?(?=[a-z])(?!(?<=\.)x(?![a-z\d]))[\w.-]+)?$/i',
+            '/^(?:==?)?\s*v?(\d+(?:\.\d+)*)(-[\w.-]+|[._]?(?=[a-z])(?!(?<=\.)x(?![a-z\d]))[\w.-]+)?$/i',
             $origin,
             $exact,
         ) === 1) {
@@ -204,7 +208,7 @@ public function __toString(): string
     private static function preReleaseBounds(string $constraint): array
     {
         \preg_match_all(
-            '/(?:^|[\s,|])[<>=!~^]*v?\d+(?:\.\d+)*(?:\+\d+)?(' . Range::preReleasePattern() . ')(?=$|[\s,|])/i',
+            '/(?:^|[\s,|])[<>=!~^]*v?\d+(?:\.\d+)*(' . Range::preReleasePattern() . ')(?=$|[\s,|])/i',
             $constraint,
             $matches,
         );

src/Module/Version/Range.php

@@ -14,6 +14,7 @@
  * - Hyphen ranges: `1.0 - 2.0`
  * - AND by a space or a comma, OR by `||` or `|`: `>=1.0 <2.0 || ^3.0`
  * - A `v` prefix, pre-releases and numeric tails in bounds: `v1.2.3`, `^3.5.0-beta.1`, `^2.0.0rc1`, `>=1.0.0-1`
+ * - Build metadata, which does not count: `>=1.2.3+build.5`, `^1.2.3-beta.1+5`
  *
  * Any count of number parts is supported on both sides, and trailing zero parts do not count.
  *
@@ -88,7 +89,7 @@ private static function parseGroup(string $group, string $constraint): array
     {
         $group === '' and throw new \InvalidArgumentException("Empty alternative in the version constraint `{$constraint}`.");
 
-        $version = '(v?\d+(?:\.\d+)*(?:\+\d+)?(?:' . self::preReleasePattern() . ')?)';
+        $version = '(v?\d+(?:\.\d+)*(?:' . self::preReleasePattern() . ')?(?:' . Version::buildPattern() . ')?)';
         if (\preg_match("/^{$version}\\s+-\\s+{$version}$/i", $group, $matches) === 1) {
             return self::parseHyphenRange($matches[1], $matches[2]);
         }
@@ -193,7 +194,7 @@ private static function parseHyphenRange(string $from, string $to): array
     private static function parseVersion(string $version): ?array
     {
         # Build metadata does not count, so the number goes without it
-        $pattern = '/^v?(\d+(?:\.\d+)*)(?:\+\d+)?(' . self::preReleasePattern() . ')?$/i';
+        $pattern = '/^v?(\d+(?:\.\d+)*)(' . self::preReleasePattern() . ')?(?:' . Version::buildPattern() . ')?$/i';
         if (\preg_match($pattern, $version, $matches) !== 1) {
             return null;
         }

src/Module/Version/Version.php

@@ -13,10 +13,10 @@
  */
 class Version implements \Stringable
 {
-    protected const VERSION_SEMVER_NUMBER_PATTERN = 'v?(\d+\.\d+\.\d+(?:\.\d+)*(?:\+\d+)?)';
+    protected const VERSION_SEMVER_NUMBER_PATTERN = 'v?(\d+\.\d+\.\d+(?:\.\d+)*)';
 
     /** A suffix starting with a letter may follow the number right away: `2.0.0rc1`, `1.2.3_beta2`. */
-    protected const VERSION_FALLBACK_PATTERN = 'v?(\d+(?:\.\d+)*(?:\+\d+)?)((?:[-+.]|(?=[a-z_]))[\w.-]+)?';
+    protected const VERSION_FALLBACK_PATTERN = 'v?(\d+(?:\.\d+)*)((?:[-+.]|(?=[a-z_]))[\w.-]+)?';
 
     protected const VERSION_HASH_SUFFIX_PATTERN = '(?:#([a-f0-9]{6,40}))?';
 
@@ -38,17 +38,39 @@ final protected function __construct(
         private readonly bool $withoutKeyword = false,
     ) {}
 
+    /**
+     * Regular expression of SemVer build metadata: a plus and dot-separated identifiers of letters, digits
+     * and hyphens, like `+build.5` or `+5-beta.1`.
+     *
+     * @return non-empty-string
+     */
+    public static function buildPattern(): string
+    {
+        return '\+[0-9a-z-]+(?:\.[0-9a-z-]+)*';
+    }
+
     /**
      * Parses a version string into its components.
      *
+     * Build metadata at the end, see {@see buildPattern()}, is dropped as in SemVer: it means nothing for
+     * the order, the stability or the suffix, so `1.2.3-linux+build.5` is `1.2.3-linux`. It stays only
+     * in {@see $string}. A plus that starts no metadata, as in `1.2.3+linux_x64`, starts a suffix.
+     *
      * @param non-empty-string $string Version string to parse
      */
     public static function fromVersionString(string $string): static
     {
+        $version = (string) \preg_replace(
+            '/' . self::buildPattern() . '(?=' . self::VERSION_HASH_SUFFIX_PATTERN . '$)/i',
+            '',
+            $string,
+            1,
+        );
+
         // Parse the version number
         \preg_match(
             '/^' . self::VERSION_FALLBACK_PATTERN . self::VERSION_HASH_SUFFIX_PATTERN . '$/i',
-            $string,
+            $version,
             $parts,
         ) or throw new \InvalidArgumentException(
             "Failed version string: {$string}.",
@@ -111,7 +133,20 @@ public function compare(self $other): int
      */
     public function compareNumber(self $other): int
     {
-        return \version_compare($this->comparableNumber(), $other->comparableNumber());
+        $parts = $this->comparableParts();
+        $otherParts = $other->comparableParts();
+        foreach ($parts as $i => $part) {
+            if (!isset($otherParts[$i])) {
+                return 1;
+            }
+
+            $byPart = self::compareDigits($part, $otherParts[$i]);
+            if ($byPart !== 0) {
+                return $byPart;
+            }
+        }
+
+        return \count($parts) <=> \count($otherParts);
     }
 
     public function __toString(): string
@@ -173,8 +208,7 @@ private static function compareTail(?string $suffix, ?string $other): int
             $isNumber = \ctype_digit($token);
             $byToken = match (true) {
                 $isNumber !== \ctype_digit($otherToken) => $isNumber ? 1 : -1,
-                # Digit strings of any length: as integers, long ones would overflow
-                $isNumber => (\strlen($token) <=> \strlen($otherToken)) ?: \strcmp($token, $otherToken) <=> 0,
+                $isNumber => self::compareDigits($token, $otherToken),
                 default => \strcmp($token, $otherToken) <=> 0,
             };
             if ($byToken !== 0) {
@@ -185,6 +219,16 @@ private static function compareTail(?string $suffix, ?string $other): int
         return \count($tokens) <=> \count($otherTokens);
     }
 
+    /**
+     * Compares digit strings of any length without leading zeros: as integers, long ones would overflow.
+     *
+     * @return int<-1, 1>
+     */
+    private static function compareDigits(string $digits, string $other): int
+    {
+        return (\strlen($digits) <=> \strlen($other)) ?: \strcmp($digits, $other) <=> 0;
+    }
+
     /**
      * Words in lower case and numbers without leading zeros, like `linux`, `x`, `64` and `1` of `Linux_x64.01`.
      *
@@ -203,16 +247,26 @@ private static function tokens(?string $suffix): array
     /**
      * A numeric suffix without a stability keyword orders as a part of the number: `1.0.0-1 > 1.0.0`.
      * With a keyword it does not: the `.2` of `2.0.0-beta.1.2` belongs to the pre-release.
-     * Trailing zero parts do not count: `1.2.3`, `1.2.3.0` and `1.2.3.0.0` are one number,
-     * nor does build metadata, as in SemVer: `1.2.3+5` is `1.2.3`.
+     * Leading zeros and trailing zero parts do not count: `1.2.3`, `01.2.3.0` and `1.2.3.0.0` are one number.
+     *
+     * @return list<string> Parts without leading zeros, empty for a version without a number
      */
-    private function comparableNumber(): string
+    private function comparableParts(): array
     {
-        $number = \explode('+', (string) $this->number, 2)[0];
+        if ($this->number === null) {
+            return [];
+        }
+
+        $number = $this->number;
         if ($this->withoutKeyword && $this->suffix !== null && \preg_match('/^\d+(?:\.\d+)*$/', $this->suffix) === 1) {
             $number .= '.' . $this->suffix;
         }
 
-        return (string) \preg_replace('/(?:\.0+)+$/', '', $number);
+        $parts = \array_map(static fn(string $part): string => \ltrim($part, '0') ?: '0', \explode('.', $number));
+        while (\count($parts) > 1 && \end($parts) === '0') {
+            \array_pop($parts);
+        }
+
+        return $parts;
     }
 }

tests/Unit/Module/Binary/BinaryVersionTest.php

@@ -62,7 +62,25 @@ public static function provideSemanticVersionOutputs(): \Generator
         yield 'semver with build metadata' => [
             'App version 2.3.4+20230415',
             '2.3.4+20230415',
-            '2.3.4+20230415',
+            '2.3.4',
+        ];
+
+        yield 'build metadata with words' => [
+            'tool 1.2.3+build.5',
+            '1.2.3+build.5',
+            '1.2.3',
+        ];
+
+        yield 'build metadata before other text' => [
+            'tool v1.2.3-rc.1+exp.sha.5114f85, built today',
+            '1.2.3-rc.1+exp.sha.5114f85',
+            '1.2.3',
+        ];
+
+        yield 'plus apart from the version' => [
+            'tool 1.2.3 +build.5',
+            '1.2.3',
+            null,
         ];
 
         yield 'pre-release without a separator' => [
@@ -176,7 +194,7 @@ public static function provideSemanticVersionOutputs(): \Generator
 
         yield 'build metadata after a pre-release' => [
             'v1.2.3-rc.1+build.5',
-            '1.2.3-rc.1',
+            '1.2.3-rc.1+build.5',
             '1.2.3',
         ];
 
@@ -263,6 +281,14 @@ public function preReleaseGluedToTheNumberKeepsItsStability(string $output, Stab
         Assert::same(BinaryVersion::fromBinaryOutput($output)->stability, $stability);
     }
 
+    #[DataSet(['tool 1.2.3+beta.2', Stability::Stable], 'build metadata like a beta')]
+    #[DataSet(['tool 1.2.3-rc.1+build.5', Stability::RC], 'release candidate before build metadata')]
+    #[Test]
+    public function buildMetadataDoesNotChangeTheStability(string $output, Stability $stability): void
+    {
+        Assert::same(BinaryVersion::fromBinaryOutput($output)->stability, $stability);
+    }
+
     #[DataSet(['tool 1.2.3beta2', '1.2.3', Stability::Beta, 2], 'beta word with a number')]
     #[DataSet(['tool 2.0.0alpha', '2.0.0', Stability::Alpha, 0], 'alpha word')]
     #[Test]

tests/Unit/Module/Binary/Internal/AbstractBinaryTest.php

@@ -0,0 +1,56 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Internal\DLoad\Tests\Unit\Module\Binary\Internal;
+
+use Internal\DLoad\Module\Binary\BinaryVersion;
+use Internal\DLoad\Module\Binary\Internal\AbstractBinary;
+use Internal\DLoad\Module\Binary\Internal\BinaryExecutor;
+use Internal\DLoad\Module\Config\Schema\Embed\Binary as BinaryConfig;
+use Internal\DLoad\Service\Logger;
+use Internal\Path;
+use Testo\Assert;
+use Testo\Codecov\Covers;
+use Testo\Data\DataProvider;
+use Testo\Test;
+
+#[Covers(AbstractBinary::class)]
+final class AbstractBinaryTest
+{
+    public static function provideVersionOutputs(): \Generator
+    {
+        yield 'build metadata' => ['App version 2.3.4+20230415', '2.3.4+20230415'];
+        yield 'pre-release with build metadata' => ['tool v1.2.3-rc.1+build.5', '1.2.3-rc.1+build.5'];
+        yield 'plain version' => ['tool 1.2.3', '1.2.3'];
+        yield 'no version' => ['no version here', null];
+    }
+
+    #[DataProvider('provideVersionOutputs')]
+    #[Test]
+    public function versionStringShowsTheWholeVersion(string $output, ?string $expected): void
+    {
+        $binary = new class('tool', new BinaryConfig(), new BinaryExecutor(new Logger()), $output) extends AbstractBinary {
+            public function __construct(
+                string $name,
+                BinaryConfig $config,
+                BinaryExecutor $executor,
+                private readonly string $output,
+            ) {
+                parent::__construct($name, $config, $executor);
+            }
+
+            public function getVersion(): BinaryVersion
+            {
+                return BinaryVersion::fromBinaryOutput($this->output);
+            }
+
+            public function getPath(): Path
+            {
+                return Path::create('tool');
+            }
+        };
+
+        Assert::same($binary->getVersionString(), $expected);
+    }
+}