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

Inconsistency in output of versions for 'show' command #7945

Open
miguelrs opened this issue Feb 1, 2019 · 8 comments
Open

Inconsistency in output of versions for 'show' command #7945

miguelrs opened this issue Feb 1, 2019 · 8 comments
Labels
Milestone

Comments

@miguelrs
Copy link

miguelrs commented Feb 1, 2019

TL;DR: See fix needed in bold in #7945 (comment)

My composer.json:

{
    "name": "miguelr/test",
    "require": {
        "symfony/symfony": "~2.8.0",
        "doctrine/doctrine-migrations-bundle": "@dev"
    }
}

Output of composer diagnose is all OK.

When I run this command:

composer show -D -o doctrine/doctrine-migrations-bundle

I get the following output, with 1.3.x-dev as version and dev-master as latest:

name     : doctrine/doctrine-migrations-bundle
descrip. : Symfony DoctrineMigrationsBundle
keywords : dbal, migrations, schema
versions : * 1.3.x-dev
latest   : dev-master
...

However, when I run this command:

composer show -D -o

I get the following output, with 1.3.x-dev 49fa399 as version and dev-master 40e9ac0 as latest version:

doctrine/doctrine-migrations-bundle 1.3.x-dev 49fa399 dev-master 40e9ac0 Symfony DoctrineMigrationsBundle
...

I inspected the code a little bit, and I think the problem is for the first output it's using getPrettyVersion() for the current and for the latest version:

$installedVersion = $package->getPrettyVersion();

$io->write('<info>latest</info> : <'.$style.'>' . $latestPackage->getPrettyVersion() . '</'.$style.'>');

However, for the second output it's using getFullPrettyVersion() for the current and for the latest version:

$packageViewData['version'] = $package->getFullPrettyVersion();

$packageViewData['latest'] = $latestPackage->getFullPrettyVersion();

Is there a reason for this difference? Couldn't it use getPrettyVersion() in both cases?


PS: FYI this bothers me because I was writing a test composer plugin where I was parsing the output of show -D -o -f json, and when I want to use the versions to find the packages with:

RepositoryManager:findPackage($data['name'], $data['version'])

it fails to parse the constraint with:

Could not parse version constraint 49fa399: Invalid version string "49fa399"
@miguelrs miguelrs changed the title Show command inconsistency in output for latest version Inconsistency in output of versions for 'show' command Feb 2, 2019
@alcohol
Copy link
Member

alcohol commented Feb 4, 2019

I do not know if there is a reason for this inconsistency. It does not seem intentional to me, but I could be mistaken.

@miguelrs
Copy link
Author

miguelrs commented Feb 4, 2019

👍 I did a bit of research and it looks like getPrettyVersion was used in both cases in the past, but it got changed as part of these commits:

  • Change in output of version: f634c69
  • Change in output of latest: 54d86eb

@miguelrs
Copy link
Author

miguelrs commented Feb 4, 2019

Also, as for the last bit of the initial message, is that another issue actually, or is that expected?
i.e., should RepositoryManager:findPackage work fine when passing something like dev-master 40e9ac0 as version, or should it throw an exception as it's doing atm?

@alcohol
Copy link
Member

alcohol commented Feb 4, 2019

Honestly we do not support using Composer as a library. So I'll refrain from commenting on that.

@miguelrs
Copy link
Author

miguelrs commented Feb 4, 2019

👌 pity, because that makes it more difficult to write plugins...

Anyway, as for the inconsistency in the output then - do you think it makes sense to open a PR? or wait for more comments? or it's just something nobody cares about? 😅

@alcohol
Copy link
Member

alcohol commented Feb 4, 2019

Yeah I can understand that it is a pity. But the amount of questions we would open ourselves up to would just explode exponentially. We already have a hard time keeping up with all the issues and pull requests we receive just for basic Composer usage.

@Seldaek do you know if there is a particular reason we use different output formats?

@miguelrs I am going to assume for now that there is no reason, so feel free to submit a pull request that makes the output consistent at least (I'd favor the more detailed output -- I am sure you can reduce it to the bare minimum that you need).

@Seldaek
Copy link
Member

Seldaek commented Feb 8, 2019

IMO using getFullPrettyVersion makes sense in the outdated listing, because you need to see clearly that there is a commit difference between current and latest in case you are on dev-master already..

In the regular composer show output for a single package, the versions lists all available versions in case of -a, and in that case having getFullPrettyVersion would be very messy IMO as a single version could result in two items in the versions list..

What I see though is that the json format probably should use getPrettyVersion instead of the getFullPrettyVersion, and it possibly should include the full source reference as well for completeness.. but I am not sure how easy a fix that is.

@miguelrs
Copy link
Author

👌 thanks for the update - I'll try to take a look and make a PR if possible

@Seldaek Seldaek added Bug and removed Question labels Oct 15, 2020
@Seldaek Seldaek added this to the Bugs milestone Oct 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants