Skip to content
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.

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

Allow Symfony 4.0 in composer.json #1420

Closed
wants to merge 2 commits into from

Conversation

pgodel
Copy link

@pgodel pgodel commented Nov 18, 2017

Q A
Bug fix? No
New feature? No
BC breaks? No
Deprecations? No
Fixed tickets N/A

#SymfonyConHackday2017

Do not forget to add notes about your changes to CHANGELOG.md

  • Add description under added/changed/fixed section.
  • Add reference to closed issues [#000].
  • Add link to issue in the end of document.

@antonmedv
Copy link
Member

Does symfony 4.0 introduce BC?

@Adirelle
Copy link

Adirelle commented Nov 23, 2017

It theoretically could. All BC are first introduced as deprecations in the previous versions (e.g. 3.*). If deployer does not raise any deprecation notice with the 3.4 symfony components, it should be ok.

@antonmedv
Copy link
Member

Okay, will release new major v7 and will merge it there — more save approach.

@antonmedv antonmedv added this to the 7.0 milestone Nov 29, 2017
"symfony/finder": "~2.7|~3.0",
"symfony/process": "~2.7|~3.0",
"symfony/yaml": "~2.7|~3.0"
"symfony/console": "~2.7 || ~3.0 || ~4.0",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Version specification is wrong.

x "~2.7 || ~3.0 || ~4.0"
o "~2.7|~3.0|~4.0"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

both are correct and work.

@tehplague
Copy link

Besides these obvious changes, please also consider the few rather subtle changes that come with Symfony 4:

  • The default log directory is now %kernel.project_dir%/var/log (note the missing "s" at the end compared to previous versions). We should default to Symfony's default conventions (see the corresponding Symfony Flex recipe https://github.com/symfony/recipes/blob/master/symfony/framework-bundle/3.3/src/Kernel.php).
  • The environment-based configuration makes prepending SYMFONY_ENV=<...> or appending --env=<...> to the CLI invocations unnecessary. As for the part of loading the application environment during Deployer deployments: we have found explicitly sourceing the variable declarations before the Symfony CLI commands to be much more reliable than to rely on the target shell to load the destination profile correctly. This might be a system configuration issue, though. However, it might generally be a good idea to provide a mechanism that allows arbitrary files on the deployment target to be sourced into Deployer's SSH session and to allow this to be configured in the server inventory. This saves from overriding most of the base Deployer tasks ;)

@antonmedv
Copy link
Member

The environment-based configuration makes prepending SYMFONY_ENV=<...> or appending --env=<...> to the CLI invocations unnecessary. As for the part of loading the application environment during Deployer deployments: we have found explicitly sourceing the variable declarations before the Symfony CLI commands to be much more reliable than to rely on the target shell to load the destination profile correctly. This might be a system configuration issue, though. However, it might generally be a good idea to provide a mechanism that allows arbitrary files on the deployment target to be sourced into Deployer's SSH session and to allow this to be configured in the server inventory. This saves from overriding most of the base Deployer tasks ;)

@tehplague can you be more specified on this one. Maybe you can show some examples?

@tehplague
Copy link

tehplague commented Dec 6, 2017

Sure. Well, my current implementation is rather "hacky" but works very well. It also provides support for Webpack Encore in the projects.

We have inventory entries of the following form:

.base: &base
  repository: 'git@gitlab.com:some/project.git'
  identity_file: identityFile
  forwardAgent: true
  bin/php: /usr/bin/php7.1

staging:
  <<: *base
  hostname: some.host.name
  user: someuser
  deploy_path: /path/to/application
  http_group: somegroup
  stage: staging
  env_cmdline:  'source /etc/someapp/app_profile &&'

and have the following Deployer configuration:

<?php

use function Deployer\{
    commandExist, server, task, run, runLocally, set, get, add, before, after, inventory, input, upload
};

use Symfony\Component\Yaml\Yaml;

require 'recipe/symfony3.php';

// Servers
inventory('build/build.properties.yml');

// Used to replace lots of set() invocations
task('load:config', function () {
    $input = input();
    $stage = $input->hasArgument('stage') ? $input->getArgument('stage') : -1;

    // Configuration
    $settingsContent = Yaml::parse(file_get_contents('build/settings.yml'));
    $settings = isset($settingsContent[$stage]) ? $settingsContent[$stage] : $settingsContent['.base'];

    foreach ($settings as $key => $config) {
        set($key, $config);
    }
})->desc('Load settings.yml');

// Make sure that settings are loaded
before('deploy', 'load:config');

task('deploy:vendors', function () {
    if (!commandExist('unzip')) {
        writeln('<comment>To speed up composer installation setup "unzip" command with PHP zip extension https://goo.gl/sxzFcD</comment>');
    }
    run('cd {{release_path}} && {{env_cmdline}} {{bin/composer}} {{composer_options}}');
});

task('frontend:build', function () {
    runLocally('yarn install');
    runLocally('yarn run build');
    runLocally('rsync -azP public/dist {{user}}@{{hostname}}:{{release_path}}/public');
})
    ->desc('Build frontend assets');

task('fpm:restart', function() {
    run('sudo service php7.2-fpm reload');
});

/**
 * Clear Cache
 */
task('deploy:cache:clear', function () {
    run('{{env_cmdline}} {{bin/php}} {{bin/console}} cache:clear {{console_options}} --no-warmup');
})->desc('Clear cache');
/**
 * Warm up cache
 */
task('deploy:cache:warmup', function () {
    run('{{env_cmdline}} {{bin/php}} {{bin/console}} cache:warmup {{console_options}}');
})->desc('Warm up cache');
/**
 * Migrate database
 */
task('database:migrate', function () {
    run('{{env_cmdline}} {{bin/php}} {{bin/console}} doctrine:migrations:migrate {{console_options}} --allow-no-migration');
})->desc('Migrate database');

/**
 * Main task
 */
task('deploy', [
    'deploy:info',
    'deploy:prepare',
    'deploy:release',
    'deploy:update_code',
    'deploy:clear_paths',
    'deploy:create_cache_dir',
    'deploy:shared',
    'deploy:vendors',
    'frontend:build',
    'deploy:cache:clear',
    'deploy:cache:warmup',
    'database:migrate',
    'deploy:symlink',
    'fpm:restart',
    'cleanup',
])->desc('Deploy application');

@antonmedv
Copy link
Member

So you need something to configure params per stage? Like this:

stage('production', function () {
    set('key', 'value');
    // ...
    inventory('deploy/production.yml');
});

@tehplague
Copy link

No, this is already handled by our settings.yml which allows per-stage configurations. We needed to be able to prepend some command to both Composer and the Symfony CLI which is why I needed to override all the tasks.
The result works practically as a "server-side version" of the env() function since by using .env the contents of the env variables are no longer known to the CI server but only to the target.

@tehplague
Copy link

Another possibility would be a sane approach to load a user profile on the destination host which would need the inventory and the SSH client interface to be extended. This would enable other features such as setting umask limits, etc. during deployments.

@Adirelle
Copy link

Adirelle commented Dec 6, 2017

@antonmedv To be clear: Symfony moved towards an environment-based configuration since 3.3. They have dropped the parameters.yml file in favor of several environment variables. In development environment, you put all of them in an local .env file and Symfony will automatically load them for convenience. But when it comes to production servers, you have to define these variables before executing any composer or console command. (I do not want to discuss whether this design choice is good or not, I just describe it).

Hence we need a way to easily define a lot of environment variables before executing every local or remote command. The first that come in mind is the "source" shell built-in.

(And more generally a way to prepend some string before every single command would be welcome, IMO)

@antonmedv
Copy link
Member

(And more generally a way to prepend some string before every single command would be welcome, IMO)

Now you can do it only for env vars.

Let's think of how we can create more robust deployment script for symfony.

So symfony requires a lot of environment params? Why not to create .env on server too?

@tehplague
Copy link

I'm not absolutely sure about the rationale behind not recommending Dotenv for production (which is officially discouraged: http://symfony.com/doc/current/components/dotenv.html). It might be due to performance considerations (manually parsing the env file on each request/invocation is slower than taking them directly from the process environment).

Nevertheless, the Dotenv component is generally unavailable on production since it is installed as a dev dependency by default. So do not expect many projects to have it available for that purpose.

@zorn-v
Copy link

zorn-v commented Dec 8, 2017

So symfony requires a lot of environment params?

Any sensitive (like passwords) or platform specific parameters usually go there.

@antonmedv
Copy link
Member

What about creating .env file in {{release_path}} and prepending each command with

source .env && ...

@zorn-v
Copy link

zorn-v commented Dec 8, 2017

It is created automatically with composer install from .env.dist if not exists
But if you put it in shared files (like in #1440) it created before and stay blank... But anyway you should put proper parameters there.
Simple source does not help coz vars should be exported. You should do something like this https://unix.stackexchange.com/a/79077
set -a; source .env; set +a probably. Need to test.

Hint: If APP_ENV env var exists symfony does not load .env with DotEnv. I was do a trick - set env to ['APP_ENV'=>get(''symfony_env)] end after "deploy:vendors" clear env (I moved DotEnv from require-dev to require). APP_ENV need for scripts executing in composer install Composer does not know anything about .env and try to load bundles for dev (default symfony env) which are not installed (if using composer install --no-dev)

@tehplague
Copy link

@zorn-v I still don't consider moving DotEnv to require as a valid option here, even if it worked for you. This would go against Symfony's recommendations. We should try to deploy Symfony applications according to their conventions. At least Deployer should come with the recipe to do so which you can then customize/override if needed.

@antonmedv
Copy link
Member

So now Deployer has env param. Can we describe all needed config there?

@zorn-v
Copy link

zorn-v commented Dec 8, 2017

@tehplague I did not say that my solution is right. I just do it coz it was convenient for me (using DotEnv in prod instead of duplicate all vars in webserver and console).
@antonmedv Yes. You can get all vars locally from some file that not under version control (to not push sensitive data to repo) for example.

@Adirelle
Copy link

Adirelle commented Dec 8, 2017

@antonmedv I imagine something like this:

# Load environment variable from '/path/to/.env/file'
set('symfony_env_file', '/path/to/.env/file');

Resulting in prepending set -a; . {{ symfony_env_file }}; set +a; to every symfony and composer commands.

I cannot remember if source is a bash-specific built-in or not, so . should work in most case.

@tehplague
Copy link

@Adirelle: . is POSIX-compliant while source is not mandated.

@antonmedv
Copy link
Member

It's okay - deployer posix dependent.

@zorn-v
Copy link

zorn-v commented Dec 8, 2017

@antonmedv I think . is better just because it should work everywere (as people says). But not at windows anyway 😄

@antonmedv
Copy link
Member

Deployer supports windows only as client machine, not target.

@inverse
Copy link

inverse commented Jan 26, 2018

When will this get merged? I guess after this It'll be worth having a symfony 4 recipe.

@gl3n
Copy link

gl3n commented Jan 30, 2018

What about merging this PR ?

@pgodel there are conflicts on the branch, and as @ttsuru said : version specification seems to be wrong.

@antonmedv
Copy link
Member

I'll ty it figure out what's best to do really soon :)

@inverse
Copy link

inverse commented Jan 30, 2018

@antonmedv might be relevant https://pantheon.io/blog/highest-lowest-testing-multiple-symfony-versions

@antonmedv
Copy link
Member

Already doing.

@xyNNN
Copy link
Contributor

xyNNN commented Feb 8, 2018

What's the current state of this pull request? I would love to use deployer with my new Symfony 4 application. Can I support in something?

@bambamboole
Copy link

We need this PR because you can't install deployer as a composer dependency in Laravel 5.6 because it uses symfony/console 4.0.4 and deployer wants 3.4.4 which is not satisfiable...

@Adirelle
Copy link

Adirelle commented Feb 10, 2018

I ended up installing the Deployed phar from deployer.org to avoid any conflict. Unfortunately, the provided SHA1 cannot be easily extracted, which prevents automated secure installation.

It would be nice to have Deployer available through something like https://phar.io/ (But maybe it is another issue.)

@inverse
Copy link

inverse commented Feb 10, 2018

@Adirelle Def a work around for now! thnx for pointing out phar.io. Will submit a issue for getting on phar easily.

@staabm
Copy link
Contributor

staabm commented Feb 10, 2018

See https://deployer.org/manifest.json for sha hashes

@antonmedv antonmedv mentioned this pull request Feb 14, 2018
@antonmedv
Copy link
Member

Closing this issue (#1559 was merged).

@antonmedv antonmedv closed this Feb 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet