Skip to content

Conversation

GrahamCampbell
Copy link
Member

This will stop people screwing up installations.

@taylorotwell
Copy link
Member

Explain?

@GrahamCampbell
Copy link
Member Author

GrahamCampbell commented Feb 14, 2018

If someone generates a lock file on PHP 7.2, then tries to deploy to a sever running PHP 7.1, then they're in for a mess if they have resolved depencies that require PHP 7.2. Setting the platform config like this makes sure that dependencies are resolved as if the PHP version was 7.1.3.

This has particular importance when it comes to Laravel 5.5 apps, where Symfony 4 may get installed as dependencies of Symfony 3 direct dependencies.

If users want to only target 7.2.0, then the can just update the mom version and the platform config accordingly.

@arubacao
Copy link
Contributor

arubacao commented Feb 14, 2018

If someone generates a lock file on PHP 7.2, then tries to deploy to a sever running PHP 7.1, then they're in for a mess if they have resolved depencies that require PHP 7.2.

Doesn't composer throw a good enough error in this case?

@taylorotwell
Copy link
Member

Wouldn't we want to set the platform to the latest stable version of PHP? Even though are minimum requirement is 7.1.3 it seems like faking the platform to that version could artificially limit us to older packages if we are running on 7.2?

@GrahamCampbell
Copy link
Member Author

Wouldn't we want to set the platform to the latest stable version of PHP? Even though are minimum requirement is 7.1.3

No, the best thing to do is to set it to the minimum version, otherwise the lock file is a lie, with respect to the minimum required version. If we want to set the version to 7.2.0, then I think we should also update the required php version in this composer.json too, which may not be a bad thing.

it seems like faking the platform to that version could artificially limit us to older packages if we are running on 7.2?

No, quite the opposite. I think this makes users think properly about their minimum required version. If they need php 7.2, or want to only support 7.2 in their app, then that's great - they can simply update the version to 7.2.0.


So, if you are agreed on the idea in principle, it remains to decide weather we go for 7.1.3 or 7.2.0, and if we keep the minimum version the same as the platform version - I'd strongly recommend this.

Another idea - what if the laravel installer were to set the minimum version of php and the platform version to the version of php used to generate the app, or 7.1.3, whichever is highest? Might this be confusing for users to have it do this, or good, because it matches their actual php version?

@taylorotwell taylorotwell merged commit 8f75f4c into master Feb 14, 2018
@arubacao
Copy link
Contributor

@GrahamCampbell Will you be using this platform configuration in your applications?

@GrahamCampbell GrahamCampbell deleted the defaults-56 branch February 15, 2018 07:41
@GrahamCampbell
Copy link
Member Author

@GrahamCampbell Will you be using this platform configuration in your applications?

Yes, already have been for over a year. :)

@balping
Copy link
Contributor

balping commented Feb 17, 2018

This is an anti-feature. People should not be responsible for checking each line of the composer.json file.

@laurencei
Copy link
Contributor

@axlon @balping - I've submitted a PR to revert this change. Feel free to continue the discussion there if needed.

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

Successfully merging this pull request may close these issues.

6 participants