Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Version of the root package is displayed incorrectly #3

Closed
morozov opened this issue Nov 20, 2017 · 11 comments
Closed

Version of the root package is displayed incorrectly #3

morozov opened this issue Nov 20, 2017 · 11 comments

Comments

@morozov
Copy link

morozov commented Nov 20, 2017

↪ wget https://github.com/phpstan/phpstan/releases/download/0.8.5/phpstan.phar

↪ php phpstan.phar --version
PHPStan - PHP Static Analysis Tool 0.8.5.0

The tag name is 0.8.5 according to the URL but the displayed version is 0.8.5.0. The same can be seen with any other application using \Jean85\PrettyVersions::getVersion('vendor/package')->getPrettyVersion(); where the vendor/package is the app itself.

Looks like when building the version from a git tag, composer/semver pads the version with zeros to have at least 4 numbers (see composer/semver@c1f064e#diff-b25d417df293a1cf205697536aed0410R33).

Not sure if the original tag can be restored from the padded version, but I'd say that most of the packages have 3 numbers in their version, so it should be safe to cut off the trailing zero. Unless there's a better solution.

@Jean85
Copy link
Owner

Jean85 commented Nov 20, 2017

Umh, this is really strange, because the issue seems to be happening only with the PHAR. I have the same version installed on some of my project, and this is the output:

$ phpstan --version
PHPStan - PHP Static Analysis Tool 0.8.5

@morozov
Copy link
Author

morozov commented Nov 20, 2017

@Jean85 if the same version is installed as a composer dependency, the issue will be not reproducible because it's not a root package in this case. If you start a new git repo and tag a version, then even without packaging a PHAR you'll see the same issue.

git init
cat > composer.json <<EOF
{
    "name": "test/test",
    "require": {
        "jean85/pretty-package-versions": "^1.0.2"
    }
}
EOF

cat > main.php  <<EOF
<?php

require __DIR__ . '/vendor/autoload.php';
echo \Jean85\PrettyVersions::getVersion('test/test')->getPrettyVersion(), PHP_EOL;
EOF

git add .
git commit -mTest
git tag 1.2.3
git checkout 1.2.3
composer install
php main.php

# 1.2.3.0

@Jean85
Copy link
Owner

Jean85 commented Nov 21, 2017

Umh, that's annoying. In this way I'm not sure there's something I can leverage to discern if the requested package is the root one...

@morozov
Copy link
Author

morozov commented Nov 21, 2017

I see at least two solutions:

  1. During a package installation hook into the process and store the information about the root package
    public static function handler(Event $composerEvent)
    {
        $root = $composerEvent->getComposer()->getPackage();
    }
    Then use this when printing the version.
  2. Less reliable but easier to implement. In the current implementation of PackageVersions, the root package is always the last one.

Alternatively, the logic of building package versions could be augmented, so that instead of the value returned by $rootPackage->getVersion(), the value of $rootPackage->getPrettyVersion() or $rootPackage->getFullPrettyVersion() (see PackageInterface) was dumped and then displayed without any parsing.

That last approach looks the hardest but the more correct to me.

@Jean85
Copy link
Owner

Jean85 commented Nov 22, 2017

Both the first and last approach would require transforming this package from a simple wrapper into a composer plugin on its own, re-doing the work of ocramius/package-versions, which is not ideal; approach number two is not reliable... I'll dig into the normalization to see if I'm able to intercept all the meaningful cases.

@Jean85
Copy link
Owner

Jean85 commented Nov 22, 2017

I've opened Ocramius/PackageVersions#51 that may solve the root cause of this issue.

@Jean85
Copy link
Owner

Jean85 commented Nov 30, 2017

Fixed with the 1.0.3 release.

@morozov
Copy link
Author

morozov commented Nov 30, 2017

Thank you @Jean85 for taking care!

@morozov
Copy link
Author

morozov commented Dec 2, 2017

The issue seems regressed by composer/composer@5ba6d7d#diff-8b93719582669d35594f58bb11eed1d2R93. In the highlighted line, the pretty version is rebuilt from the normalized one, so the trailing zero is re-added back.

@Jean85 Jean85 reopened this Dec 4, 2017
@Jean85
Copy link
Owner

Jean85 commented Dec 4, 2017

I've opened composer/composer#6866 that should finally fix this.

@Jean85
Copy link
Owner

Jean85 commented Feb 27, 2018

composer/composer#6866 has been closed, hence this is resolved.

@Jean85 Jean85 closed this as completed Feb 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants