webimpress / composer-extra-dependency

Composer plugin to require extra dependencies
BSD 2-Clause "Simplified" License
11 stars 2 forks source link

Composer parser crashes in no-interaction mode #3

Closed wearethewinds closed 7 years ago

wearethewinds commented 7 years ago

If this module is required as a sub-dependency (e.g. an own module requiring your http-middleware-compatibility module) which does not require the module in question, composer-extra-dependency adds the following line to the composer.json in --no-interaction-mode: "http-interop/http-middleware": null which causes Composer's version parser to fail:

[UnexpectedValueException]                                      
Could not parse version constraint : Invalid version string ""

This pr adds a default value to IOInterface:askAndValidate in order to fall back to the latest versio in no-interaction-mode (which basically behaves the same as leaving the question unanswered in interaction-mode). I am well aware that this can lead to conflicts, as the implementation might not match the latest version yet. On the other hand considering that non-interactive update-calls are mostly done in automated environments, I would rather appreciate an error message that points to the parts where I violate against the latest version rather than the error message posted above. I would need to add an additional version constraint for the particular packages anyway if my implementation does not match the latest implementation.

Minimum composer.json to reproduce the problem:

{
    "name": "test/test",
    "license": "BSD-2-Clause",
    "keywords": [
        "webimpress",
        "psr-15",
        "middleware"
    ],
    "require": {
        "php": "^5.6 || ^7.0",
        "webimpress/http-middleware-compatibility": "0.1.3"
    }
}

Just run composer update --no-interaction.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.03%) to 98.347% when pulling 52e4b97bcc6bd01a8e1653bbc45ac95cedd401e1 on wearethewinds:bugfix/no-interactive-fix into 8722031102594a494e4eef1221f8a1a32f82830a on webimpress:master.

michalbundyra commented 7 years ago

@wearethewinds Thanks for reporting the issue. I've provided a bit different solution which handle also case you've described - another package requires component in specific version. My solution uses * in case no-interaction mode, then proper version will be installed and composer.json will be updated with "package-name": "*"`.

As example we would have:

{
    "name": "test/test",
    "require": {
        "php": "^5.6 || ^7.0",
        "webimpress/http-middleware-compatibility": "^0.1.3",
    "zendframework/zend-expressive": "2.0.5"
    }
}

(zend-expressive 2.0.5 requires http-interop/http-middleware 0.4.1, and we would like to have installed that version, and in no-interaction mode we would like to have added http-interop/http-middleware: "*").... Hm... or rather we would prefer to not add anything. Let me think, maybe we should disable the plugin on no-interaction mode? Thoughts?

wearethewinds commented 7 years ago

Hi!

Thanks for your fast response. Really appreciate it :)

I see a valid point in both your proposed ways, which indeed solve the problem a little bit more elegant than my solution. I actually think that deactivating the plugin in --no-interaction mode is probably the most reasonable solution as we leave everything up to Composer to find the appropriate version depending on the other composer-files and probably set flags (like --prefer-lowest). Adding a "*" to the local composer.json just feels like unnecessary clutter. But it is probably the easiest way to solve as already proposed in your other PR. I'm fine with both ways.

michalbundyra commented 7 years ago

Closed in favour of #6, thanks @wearethewinds !