8000 Update setup.rst by stoccc · Pull Request #7298 · symfony/symfony-docs · GitHub
[go: up one dir, main page]
Skip to content

Update setup.rst #7298

8000
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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

Update setup.rst #7298

wants to merge 3 commits into from

Conversation

stoccc
Copy link
Contributor
@stoccc stoccc commented Dec 28, 2016

I Added a paragraph regarding php version constraints in composer.json. In fact, when installing Symfony you have to meet some production server requirements.

Without that, for first install that's ok (if you read symfony requirements you can choose the right Symfony version that's shipped with the right libraries version), but doing updates (just running composer update) may break things with libraries being updated that don't meet your server requirements (e.g. php version) or may do nothing (e.g. running composer update symfony/symfony if you have 2.8.12 installed won't install 2.8.15, despite of what is said here http://symfony.com/doc/current/setup/upgrade_patch.html because of the need of twig/twig 1.30.0 - so you would run "composer update symfony/symfony twig/twig" and so on for other such dependencies to obtain the patch update - that's not so confortable!).

Best regards.

@stoccc
Update setup.rst
ce75a7d 
I Added a paragraph regarding php version constraints in composer.json. In fact, when installing Symfony you have to meet some production server requirements. Maybe you should consider adding a page in the documentation that could better explain these need and this configuration line.

Without that, for first install that's ok (if you read symfony requirements you can choose the right Symfony version that's shipped with the right libraries version), but doing updates (just running composer update) may break things with libraries being updated that don't meet your server requirements (e.g. php version) or may do nothing (e.g. running composer update symfony/symfony if you have 2.8.12 installed won't install 2.8.15, despite of what is said here http://symfony.com/doc/current/setup/upgrade_patch.html because of the need of twig/twig 1.30.0 - so you would run "composer update symfony/symfony twig/twig" and so on for other such dependencies to obtain the patch update - that's not so confortable!).

Best regards.
@javiereguiluz
Copy link
Member

@stoccc thanks for your contribution!

The setup article is the most critical content for us, so we review anything added/changed very carefully. I hope you understand it.

Regarding your proposal, the Symfony Standard Edition (and therefore, the Symfony Installer too) already defines this option to avoid the problems you mentioned. For example: PHP 5.3.9 in Sf 2.8 (https://github.com/symfony/symfony-standard/blob/2.8/composer.json#L43) and PHP 5.5.9 in Sf 3.2 (https://github.com/symfony/symfony-standard/blob/3.2/composer.json#L48). So please, tell me how did you experience the issue of not having defined this option. Thanks!

@stoccc
Copy link
Contributor Author
stoccc commented Dec 28, 2016 •
edited
Loading

@javiereguiluz
I followed the documentation here http://symfony.com/doc/2.8/setup.html on ubuntu-gnome 16.10.

I tried again installing using the symfony installer just downloaded from here https://symfony.com/installer
I made a new test project like this
./symfony.phar new my_project 2.8
but the result is the same: there isn't such a constraint in the generated composer.json

Even if I don't specify the symfony version, the generated composer.json doesn't contain the constraint on php version.

Any idea?

Best regards

ps neither the symfony demo application contains that constraint

@xabbuh
Copy link
Member
xabbuh commented Dec 28, 2016

Can you show the complete contents of the composer.json file?

@stoccc
Copy link
Contributor Author
stoccc commented Dec 28, 2016 •
edited
Loading

@xabbuh

sure, here you are.

{
    "name": "ste/my_project",
    "license": "proprietary",
    "type": "project",
    "autoload": {
        "psr-4": {
            "": "src/"
        },
        "classmap": [
            "app/AppKernel.php",
            "app/AppCache.php"
        ]
    },
    "require": {
        "php": ">=5.3.9",
        "symfony/symfony": "2.8.*",
        "doctrine/orm": "^2.4.8",
        "doctrine/doctrine-bundle": "~1.4",
        "symfony/swiftmailer-bundle": "~2.3",
        "symfony/monolog-bundle": "~2.11.3",
        "sensio/distribution-bundle": "~5.0",
        "sensio/framework-extra-bundle": "^3.0.2",
        "incenteev/composer-parameter-handler": "~2.0"
    },
    "require-dev": {
        "sensio/generator-bundle": "~3.0",
        "symfony/phpunit-bridge": "~2.7"
    },
    "scripts": {
        "symfony-scripts": [
            "Incenteev\\ParameterHandler\\ScriptHandler::buildParameters",
            "Sensio\\Bundle\\DistributionBundle\\Composer\\ScriptHandler::buildBootstrap",
            "Sensio\\Bundle\\DistributionBundle\\Composer\\ScriptHandler::clearCache",
            "Sensio\\Bundle\\DistributionBundle\\Composer\\ScriptHandler::installAssets",
            "Sensio\\Bundle\\DistributionBundle\\Composer\\ScriptHandler::installRequirementsFile",
            "Sensio\\Bundle\\DistributionBundle\\Composer\\ScriptHandler::prepareDeploymentTarget"
        ],
        "post-install-cmd": [
            "@symfony-scripts"
        ],
        "post-update-cmd": [
            "@symfony-scripts"
        ]
    },
    "config": {
        "bin-dir": "bin"
    },
    "extra": {
        "symfony-app-dir": "app",
        "symfony-web-dir": "web",
        "symfony-assets-install": "relative",
        "incenteev-parameters": {
            "file": "app/config/parameters.yml"
        },
        "branch-alias": null
    }
}

@javiereguiluz
Copy link
Member

I'm very sorry but I was completely wrong in my previous message.

It's true that Symfony defines the config.platform setting ... but the Symfony Installer removes it after the installation. We have this line of code somewhere:

if (isset($composerConfig['config']['platform']['php'])) {
    unset($composerConfig['config']['platform']['php']);
    ...
}

Why do we do this? It's OK for Symfony to define that setting, because when we build the packages we do a composer install and we must be sure that the installed packages are for PHP 5.5. Otherwise, if the machine building the packages has PHP 7, the generated package won't be compatible with PHP 5.5.

However, if the Symfony Installer keeps this option, developers using PHP 7 in their computers/server can't install newer packages that are compatible with PHP 7 when doing a composer update.

So it seems that we need to add some note about this, as your propose here.

@stoccc
Copy link
Contributor Author
stoccc commented Jan 2, 2017

thank you for your explanation, it's a pleasure to contribute to the symfony project.

@wouterj
Copy link
Member
wouterj commented Apr 15, 2017

Hi @stoccc!

I'm very sorry for the long delay on this PR from our side. We've shortly discussed this PR in the doc team today and I've some good news: We like your idea of mentioning the fix for this common problem! However, we don't like the location of the current proposal. The setup guide is the first article people see when using Symfony. It should be super simple, super fun and to-the-point.

Setting up this version in Composer to prevent broken deployments seems more a deployment/upgrade thing. Can you please move this section to it's own sub-guide under /setup/? You can call it e.g. "How to prevent PHP conflicts when using Composer". The contents can be equal to the current section content, maybe with a small description of what this does ("setting the PHP version used by Composer to check requirements").

So in short: Perfect section, but please move it to it's own article :) If you can do that, it'll be perfect. If you no longer have the time/motivation, I perfectly understand and we can take this over (while keeping your credits of course).

@stoccc
Copy link
Contributor Author
stoccc commented Apr 23, 2017 via email

@HeahDude
Copy link
Contributor

ping @stoccc :) Would you like to finish this PR?

@stoccc
restored current setup.rst
a0786d9 
restored current setup.rst
@stoccc
Copy link
Contributor Author
stoccc commented Aug 11, 2017

mmmh looks like I did a mistake :)
i was trying to remove my changes in setup.rst and then adding a new file in setup/

@stoccc
Copy link
Contributor Author
stoccc commented Aug 11, 2017

maybe should I add a new PR and close this one?

@stoccc stoccc mentioned this pull request Aug 11, 2017
Closed
@stoccc
Copy link
Contributor Author
stoccc commented Aug 11, 2017

@wouterj
I handled your request here #8278.
I'm sorry for the delay :)

I close this PR.

@stoccc stoccc closed this Aug 11, 2017
@stoccc stoccc deleted the patch-1 branch August 11, 2017 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@stoccc @javiereguiluz @xabbuh @wouterj @HeahDude @carsonbot
0