search

magento-hackathon / magento-composer-installer

Composer installer for Magento modules
210 stars 154 forks source link

add a PHP dependency into the composer.json #150

Closed lsmith77 closed 9 years ago

lsmith77 commented 9 years ago

in an older project we were using 0.1.* and just found that 0.1.3 is using short array syntax which requires PHP 5.4.

in this spirit it would be good to at least add such a require now, so that in the future you can bump the require when the requirements increase to prevent surprising breakage for users.

Flyingmana commented 9 years ago

could you point out the place/error?

The 2.* version still works with 5.3, at least mostly. There are only a few special Features which dont work in this php version.

If you really still use it, I can add a 0.1.x branch, to remove the 5.4 requirement

lsmith77 commented 9 years ago

use of short array syntax here: https://github.com/magento-hackathon/magento-composer-installer/blob/0.1.3/src/MagentoHackathon/Composer/Magento/Plugin.php#L65

btw if the 0.1.x branch is still maintained, it might be good to open a dedicated branch.

Flyingmana commented 9 years ago

I would not call it maintained, but I dont see a reason to not fix such general things.

but, are you sure the version of the link is what you currently use? I think this points accidentally to the https://github.com/magento/magento-composer-installer repository

Explanation: I fetch several forks of this project local, and I fear the tags conflicted... So could you maybe verify the version you have?

lsmith77 commented 9 years ago

not quite sure I understand .. we are using the 0.1.2 tag now since the 0.1.3 tag gave us a syntax error on PHP 5.3.

here is the PR I did to fix things for now: https://github.com/liip/liip-magento-shared/pull/2

lsmith77 commented 9 years ago

I should note .. this stuff obviously needs to be updated. and the quick fix I did in the project was actually add that require there.

davidverholen commented 9 years ago

Hey,

I think you should not require a specific Version in a dependency itself. If another dependency would do the same thing with another Version, your Project cannot be installed anymore.

You should keep the requirement in the magento-module with the * version. If you wan't to specify this version for your project, you can require it again in your projects composer.json with the specific version you wish to have.

Flyingmana commented 9 years ago

ok, more simple words: the current tagged version 0.1.2 is not part of any branch inside this repository, you probably have code from a fork

lsmith77 commented 9 years ago

the dependency on 0.1.2 is a hack to get around the fact that 0.1.3 (accidentally) increased the php version requirement. ideally I would have a "0.1.*" requirement in this case.

as for the 0.1.3 release, its listed on: https://packagist.org/packages/magento-hackathon/magento-composer-installer

and for the record, its also listed here: https://packagist.org/packages/magento/magento-composer-installer

lsmith77 commented 9 years ago

as well as 0.1.2

davidverholen commented 9 years ago

the https://packagist.org/packages/magento-hackathon/magento-composer-installer was created from a fork which never really worked.

It's abandoned now and the urls seem to point to the correct repository. But maybe some Version references are still wrong

The https://packagist.org/packages/magento/magento-composer-installer is the fork from the magento team which is currently used to install magento2 components

lsmith77 commented 9 years ago

ok ... in the end we wanted a 1.* release after all. however it would still be good to add PHP depdenencies

Flyingmana commented 9 years ago

have added the version requirement in master(min 5.3.2 as composer itself has) and also added a 1.3.x branch.

Feel free to report cases where features are not compatible yet with 5.3