PHPCSStandards / composer-installer

Composer installer for PHP_CodeSniffer coding standards
https://packagist.org/packages/dealerdirect/phpcodesniffer-composer-installer
MIT License
552 stars 36 forks source link

Fixed v4 constraint #115

Closed GrahamCampbell closed 3 years ago

GrahamCampbell commented 4 years ago

4.0.x-dev is the name of a branch, and not a version constraint. You should use a proper version constraint, otherwise anyone else who does use a one will find composer doesn't know what to do.

jrfnl commented 4 years ago

@GrahamCampbell That's because 4.x isn't a tag, it's a development branch and this was added to allow testing with that branch. IIRC using 4.x won't work until there is a release. (Can't test at the moment)

GrahamCampbell commented 4 years ago

No, it will work fine. That's the nice thing about composer. It knows how to install 4.0.x dev versions.

jrfnl commented 4 years ago

@GrahamCampbell Thanks. I'll have a play with it again once I'm behind a proper computer to test it.

Potherca commented 4 years ago

@jrfnl Should I just merge and deploy it as 0.7.1?

jrfnl commented 4 years ago

Sorry for the delay. I've finally had some time to think of some test scenarios and have a play with it.

Scenario 1 - plain install for the plugin itself:

With both the 0.7.0 release as well as with this PR, PHPCS 3.5.5 is installed as expected and desired ("prefer-stable": true).

Scenario 2 - minimal external project setup for PHPCS 3.x (stable)

Note: PHPCSUtils also already allows for PHPCS 4.x and contains external standards which should trigger the plugin.

{
    "name": "jrfnl/plugin-issue-115",
    "require": {
        "php": ">=5.4",
        "squizlabs/php_codesniffer": "^3.1",
        "phpcsstandards/phpcsutils" : "^1.0",
        "dealerdirect/phpcodesniffer-composer-installer": "^0.7.0"
    },
    "minimum-stability": "dev",
    "prefer-stable": true
}

Run composer install - output as expected & correct:

Package operations: 3 installs, 0 updates, 0 removals
  - Installing squizlabs/php_codesniffer (3.5.5): Loading from cache
  - Installing dealerdirect/phpcodesniffer-composer-installer (v0.7.0): Loading from cache
  - Installing phpcsstandards/phpcsutils (1.0.0-alpha3): Loading from cache
Writing lock file
Generating autoload files
PHP CodeSniffer Config installed_paths set to ../../phpcsstandards/phpcsutils

Scenario 3 - minimal external project setup for PHPCS 4.x (unstable)

{
    "name": "jrfnl/plugin-issue-115",
    "require": {
        "php": ">=5.4",
        "squizlabs/php_codesniffer": "^4.0",
        "phpcsstandards/phpcsutils" : "^1.0",
        "dealerdirect/phpcodesniffer-composer-installer": "^0.7.0"
    },
    "minimum-stability": "dev",
    "prefer-stable": true
}

Run composer install - output as expected & correct:

Package operations: 3 installs, 0 updates, 0 removals
  - Installing squizlabs/php_codesniffer (4.0.x-dev 14e9ce7): Cloning 14e9ce7599 from cache
  - Installing dealerdirect/phpcodesniffer-composer-installer (v0.7.0): Loading from cache
  - Installing phpcsstandards/phpcsutils (1.0.0-alpha3): Loading from cache
Writing lock file
Generating autoload files
PHP CodeSniffer Config installed_paths set to ../../phpcsstandards/phpcsutils

otherwise anyone else who does use a one will find composer doesn't know what to do.

In other words, I have not been able to reproduce the issue this PR is supposed to solve as the test scenario did use "proper constraints", while 0.7.0 refers to the dev branch, and the results were as expected, without any problems reported by Composer about it not knowing what to do.

Tested with Composer 1.1.0.8.

@GrahamCampbell Could you provide a test scenario which actually demonstrates the problem ?

@Potherca I'm fine with merging this either way as it doesn't seem to do any harm either. Based on the above tests, I don't think a new release straight away is warranted though as things seem to work fine as they are.

jrfnl commented 4 years ago

@GrahamCampbell Just checking in - have you had a chance to come up with a reproduction scenario for this issue ?

GrahamCampbell commented 4 years ago

I think you're right that it technically works, but it's not a proper version constraint. Composer just works out that you meant to type what I changed it to in this PR.

jrfnl commented 4 years ago

@GrahamCampbell To be fair, I believe it's the other way around, though I could be wrong.

Composer supports branch-based constraints. So when using the "proper" version restraint, as there is no 4.x version available yet, composer looks for a branch which matches and finds the correct branch. So the branch constraint currently used saves Composer from the overhead of having to do that as it already points to the branch.

GrahamCampbell commented 4 years ago

I still maintain that my changes are the best way to do things, and will actually still work once a 4.x release has been tagged.

jrfnl commented 4 years ago

@GrahamCampbell And once there is a (RC) release of PHPCS 4.x, we will definitely make that change. As things stand, PHPCS 4.x is not officially supported (yet), the 4.0.x-dev allowance has, for now, only been added to make it easier for external PHPCS standards to test against PHPCS 4.x, not as an official statement of PHPCS 4.x being supported by this plugin.