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

Add better support for PHP applications #68

Closed
hajekj opened this issue Mar 15, 2017 · 13 comments

Comments

Projects
None yet
5 participants
@hajekj
Copy link

commented Mar 15, 2017

It would be great if we could produce correct build script for PHP applications (with Composer) so builds would work out of box for PHP applications - I am well aware of Composer site extension, however I think that since we have support for Node.js with NPM packages, this should be supproted at the same level.

:: 1. Composer.phar
IF EXIST "%DEPLOYMENT_TARGET%\composer.json" (
  cd %DEPLOYMENT_TARGET%

  IF NOT EXIST "%DEPLOYMENT_TARGET%\composer.phar" (
    call curl -s http://getcomposer.org/installer | php > nul
    IF !ERRORLEVEL! NEQ 0 goto error
  )

  call php composer.phar install
  IF !ERRORLEVEL! NEQ 0 goto error
)

Above, I have added a simple example which demonstrates the installation of Composer from within the build script. The best solution would be to include Composer natively to Azure App Service. I think this would be a great step forward and I can try to make a pull request which would resolve this if the maintainers agree with this suggestion.

@davidebbo

This comment has been minimized.

Copy link
Member

commented Mar 15, 2017

Right now, I don't think I've seen a complete well working PHP script that does that, and I recall when we tried a while back there were issues. If you can get a good script working, I suggest as a first step that we push it to https://github.com/projectkudu/kudu-deployment-scripts as a sample to make it easier. Then if there is enough demand we can later look at integrating it in the generator.

@davidebbo

This comment has been minimized.

Copy link
Member

commented Mar 16, 2017

Ok, I merged it in. /cc @SyntaxC4-MSFT as he has been playing with Composer in Kudu scripts in the past and may be interesting in your script. https://github.com/projectkudu/kudu-deployment-scripts/blob/master/scripts/deploy-php.cmd

@nickwalkmsft

This comment has been minimized.

Copy link
Contributor

commented Mar 16, 2017

cc @michimune, as he has been working on this as well.

@michimune

This comment has been minimized.

Copy link

commented Mar 20, 2017

@davidebbo We have the same request for Linux. Should we do push a bash script to the kudu-deployment-scripts repo, too?

@davidebbo

This comment has been minimized.

Copy link
Member

commented Mar 20, 2017

Yes, if the template works well and we want the scenario to be less manual, then updating kuduscript is the next logical step.

@hajekj

This comment has been minimized.

Copy link
Author

commented Apr 18, 2017

Just checking - any updates? @SyntaxC4-MSFT @davidebbo

@davidebbo

This comment has been minimized.

Copy link
Member

commented Apr 18, 2017

@Benjiiim, this should be covered by your recent change, right? @hajekj one thing to note is that we are now focusing the PHP effort on Linux workers (which are in Preview).

@Benjiiim

This comment has been minimized.

Copy link
Contributor

commented Apr 18, 2017

Indeed.

@hajekj

This comment has been minimized.

Copy link
Author

commented Apr 18, 2017

@Benjiiim - lovely change (5166d4f) indeed! When is this going to be deployed to production?

Actually, I noticed a slight issue with your implementation of Composer - you first run KuduSync and then install the packages - which could possibly result in higher down time of the application due to the packages getting downloaded to the production environment already? It could also possibly have some bad effect while deploying to production - file is in use error and so on.

Personally, I would prefer to see it the other way around - download composer packages into the DEPLOYMENT_SOURCE directory and then copy it over. It could also allow for the deployments to be faster if you redirect the DEPLOYMENT_SOURCE to D:\local\deployments or similar.

Additionally, does this work on regular App Service?

In the App Service on Linux PHP images (https://github.com/Azure-App-Service/php), there is no Composer preinstalled so since the script doesn't install it on its own or it is not comitted into the source control (which is bad practice I think). Maybe have the bash script download and install Composer?

@Benjiiim

This comment has been minimized.

Copy link
Contributor

commented Apr 18, 2017

The App Service on Linux PHP images doesn't contain Composer as the deployment happens in the Kudu Docker image where PHP and Composer has been added a few days ago : https://github.com/Azure-App-Service/kudu

From what I know, a new Kudu build has to be built and used in the App Service on Linux Kudu DockerFile before it can be deployed with the next App Service on Linux update. @nickwalkmsft might be able to add more details here.

On purpose, this improvement will not be triggered in the regular App Service (see https://github.com/projectkudu/kudu/blob/master/Kudu.Core/Deployment/Generator/PHPSiteEnabler.cs).
From my perspective, two reasons for that:

  • We don't want to break something in the regular App Service by pushing a new behaviour for all PHP apps deployment. Users have acceptable workarounds with the custom deployement script (manually or with the extension).
  • App Service on Linux is being built today to improve PHP/Node.js/RoR/... developpers experience on Azure. That's the right place to add these kind of improvements, especially now during preview.

To be honest, I haven't thought a lot about ordering, as I have replicated what you can find in the Ruby template or even the Node.js one which has been used for years. @davidebbo might have more insights than me on this one.

@davidebbo

This comment has been minimized.

Copy link
Member

commented Apr 18, 2017

The reason for the ordering in Node was primarily to avoid a double hit on our slow file system with huge npm trees. i.e. if we did the install in the repo folder before the kudusync, we'd end up having two copies of the tree, each with potentially many thousand files.

This may be a less concern for other stacks if the file count isn't as crazy as npm.

@hajekj

This comment has been minimized.

Copy link
Author

commented Apr 19, 2017

@Benjiiim Okay, now it makes perfect sense, thanks a lot for explaining. I believe this can be considered as resolved then.

@hajekj hajekj closed this Apr 19, 2017

@hajekj

This comment has been minimized.

Copy link
Author

commented Apr 20, 2017

@Benjiiim @davidebbo - just FYI, wrote an article about the current deployment methods - https://hajekj.net/2017/04/20/deploying-php-web-applications-to-azure-app-service/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.