Autorun composer install #95

Closed
opened 2016-11-25 13:21:44 +00:00 by SamMousa · 21 comments
SamMousa commented 2016-11-25 13:21:44 +00:00 (Migrated from github.com)

Doesn't it make sense to run composer install on start if a composer.lock is found?

Doesn't it make sense to run composer install on start if a composer.lock is found?
richarvey commented 2017-01-03 08:59:29 +00:00 (Migrated from github.com)

would this need to be run if the site was updated also?

would this need to be run if the site was updated also?
SamMousa commented 2017-01-03 09:06:21 +00:00 (Migrated from github.com)

Yes!

On Jan 3, 2017 10:03, "Ric Harvey" notifications@github.com wrote:

would this need to be run if the site was updated also?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/ngineered/nginx-php-fpm/issues/95#issuecomment-270072030,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAhYzTrPUopYt2MrKsn0XxW6MW0kmgkOks5rOg3ygaJpZM4K8bz6
.

Yes! On Jan 3, 2017 10:03, "Ric Harvey" <notifications@github.com> wrote: > would this need to be run if the site was updated also? > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <https://github.com/ngineered/nginx-php-fpm/issues/95#issuecomment-270072030>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AAhYzTrPUopYt2MrKsn0XxW6MW0kmgkOks5rOg3ygaJpZM4K8bz6> > . >
richarvey commented 2017-01-03 09:32:43 +00:00 (Migrated from github.com)

Ok where would the composer file live? Would you always include in the root? Also would it be the lock file we look for or a json file?

Ok where would the composer file live? Would you always include in the root? Also would it be the lock file we look for or a json file?
SamMousa commented 2017-01-03 10:48:34 +00:00 (Migrated from github.com)

A lock file definitely and I think it must be in the root (not sure)

On Jan 3, 2017 10:32 AM, "Ric Harvey" notifications@github.com wrote:

Ok where would the composer file live? Would you always include in the
root? Also would it be the lock file we look for or a json file?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/ngineered/nginx-php-fpm/issues/95#issuecomment-270076690,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAhYzbVJJDZB5Gu329dddfhQVRx_NC-vks5rOhW8gaJpZM4K8bz6
.

A lock file definitely and I think it must be in the root (not sure) On Jan 3, 2017 10:32 AM, "Ric Harvey" <notifications@github.com> wrote: > Ok where would the composer file live? Would you always include in the > root? Also would it be the lock file we look for or a json file? > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <https://github.com/ngineered/nginx-php-fpm/issues/95#issuecomment-270076690>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AAhYzbVJJDZB5Gu329dddfhQVRx_NC-vks5rOhW8gaJpZM4K8bz6> > . >
richarvey commented 2017-01-03 17:37:33 +00:00 (Migrated from github.com)

@SamMousa are you sure about the lock file i thought that was auto created when you ran composer:

https://daylerees.com/the-composer-lock-file/

I may be wrong here so any help greatly appreciated. From reading that on a git pull I'd run composer install if there was a composer.json file in the root directory.

@SamMousa are you sure about the lock file i thought that was auto created when you ran composer: https://daylerees.com/the-composer-lock-file/ I may be wrong here so any help greatly appreciated. From reading that on a git pull I'd run composer install if there was a composer.json file in the root directory.
SamMousa commented 2017-01-03 18:46:32 +00:00 (Migrated from github.com)

Yes it is created and contains the exact commit hash of each dependency.
For non libraries it is committed to version control (to guarantee a test
environment exactly the same as the production environment).

See
https://getcomposer.org/doc/01-basic-usage.md#composer-lock-the-lock-file

On Jan 3, 2017 18:37, "Ric Harvey" notifications@github.com wrote:

@SamMousa https://github.com/SamMousa are you sure about the lock file
i thought that was auto created when you ran composer:

https://daylerees.com/the-composer-lock-file/

I may be wrong here so any help greatly appreciated. From reading that on
a git pull I'd run composer install if there was a composer.json file in
the root directory.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/ngineered/nginx-php-fpm/issues/95#issuecomment-270173167,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAhYzbrGY4h3AYzVqiGZdzV9NfLi0saWks5rOoddgaJpZM4K8bz6
.

Yes it is created and contains the exact commit hash of each dependency. For non libraries it is committed to version control (to guarantee a test environment exactly the same as the production environment). See https://getcomposer.org/doc/01-basic-usage.md#composer-lock-the-lock-file On Jan 3, 2017 18:37, "Ric Harvey" <notifications@github.com> wrote: > @SamMousa <https://github.com/SamMousa> are you sure about the lock file > i thought that was auto created when you ran composer: > > https://daylerees.com/the-composer-lock-file/ > > I may be wrong here so any help greatly appreciated. From reading that on > a git pull I'd run composer install if there was a composer.json file in > the root directory. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <https://github.com/ngineered/nginx-php-fpm/issues/95#issuecomment-270173167>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AAhYzbrGY4h3AYzVqiGZdzV9NfLi0saWks5rOoddgaJpZM4K8bz6> > . >
richarvey commented 2017-01-04 17:19:10 +00:00 (Migrated from github.com)

We'll spin up a new branch asap with this in to test.

We'll spin up a new branch asap with this in to test.
richarvey commented 2017-01-06 14:56:51 +00:00 (Migrated from github.com)

Hi @SamMousa I've committed some code that "should" run install when you first launch the container and if you do a pull to update the code it should run an update.

Can you check this functionality behaves as expected.

Cheers Ric

Hi @SamMousa I've committed some code that "should" run install when you first launch the container and if you do a pull to update the code it should run an update. Can you check this functionality behaves as expected. Cheers Ric
SamMousa commented 2017-01-06 17:53:59 +00:00 (Migrated from github.com)

Will do next week.

On Jan 6, 2017 3:56 PM, "Ric Harvey" notifications@github.com wrote:

Hi @SamMousa https://github.com/SamMousa I've committed some code that
"should" run install when you first launch the container and if you do a
pull to update the code it should run an update.

Can you check this functionality behaves as expected.

Cheers Ric


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/ngineered/nginx-php-fpm/issues/95#issuecomment-270919824,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAhYzf7jYsvJziFRfUIxhYrE9BWQ_csIks5rPlYzgaJpZM4K8bz6
.

Will do next week. On Jan 6, 2017 3:56 PM, "Ric Harvey" <notifications@github.com> wrote: > Hi @SamMousa <https://github.com/SamMousa> I've committed some code that > "should" run install when you first launch the container and if you do a > pull to update the code it should run an update. > > Can you check this functionality behaves as expected. > > Cheers Ric > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <https://github.com/ngineered/nginx-php-fpm/issues/95#issuecomment-270919824>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AAhYzf7jYsvJziFRfUIxhYrE9BWQ_csIks5rPlYzgaJpZM4K8bz6> > . >
SamMousa commented 2017-01-09 10:23:18 +00:00 (Migrated from github.com)

@richarvey Just had a chance to look at the implementation and it doesn't work for my case.

The convention is to put composer.lock is in the project root, not in the webroot. My project(s) use a public folder as the webroot, the code you've added searches for composer.lock in this webroot directory; instead it should be searched for in the project root. I'll add some comments to the commit as well.

@richarvey Just had a chance to look at the implementation and it doesn't work for my case. The convention is to put `composer.lock` is in the project root, not in the webroot. My project(s) use a `public` folder as the webroot, the code you've added searches for composer.lock in this webroot directory; instead it should be searched for in the project root. I'll add some comments to the commit as well.
SamMousa commented 2017-01-09 10:25:03 +00:00 (Migrated from github.com)

Also note, that there is technically no need to check for the existence of a composer lock file; if you just ru composer install you'll get this in case it can't run:

$ composer install
Composer could not find a composer.json file in /tmp
To initialize a project, please create a composer.json file as described in the https://getcomposer.org/ "Getting Started" section
Also note, that there is technically no need to check for the existence of a composer lock file; if you just ru composer install you'll get this in case it can't run: ```` $ composer install Composer could not find a composer.json file in /tmp To initialize a project, please create a composer.json file as described in the https://getcomposer.org/ "Getting Started" section ````
richarvey commented 2017-01-09 10:50:02 +00:00 (Migrated from github.com)

Right I think i understand now. Let me add some variables and see if we can get this working.

Right I think i understand now. Let me add some variables and see if we can get this working.
SamMousa commented 2017-01-09 13:04:52 +00:00 (Migrated from github.com)

@richarvey Also add --no-dev, since dev dependencies shouldn't be installed on production environment (alternatively use a variable for this)

@richarvey Also add `--no-dev`, since dev dependencies shouldn't be installed on production environment (alternatively use a variable for this)
SKoschnicke commented 2017-02-27 09:59:16 +00:00 (Migrated from github.com)

I don't think this is working is intended. The startup script checks for a composer.lock file in the webroot and executes the composer in the project root then.

If webroot is not the same as project root, either the check for the composer.lock file does not work (because composer.lock is located in the project root) or the composer command does not find the composer.lock file (because the command is executed in the project root and the composer.lock is located in the webroot).

Also, the composer.phar binary needs to reside in the repository (in the project root). I don't think that it is good practice to put this binary artifact into the source repository. The image itself should come with a composer binary.

I'm happy to help with this issue, if you want. :)

I don't think this is working is intended. The startup script checks for a `composer.lock` file in the *webroot* and executes the composer in the *project root* then. If webroot is not the same as project root, either the check for the `composer.lock` file does not work (because `composer.lock` is located in the project root) or the composer command does not find the `composer.lock` file (because the command is executed in the project root and the `composer.lock` is located in the webroot). Also, the `composer.phar` binary needs to reside in the repository (in the project root). I don't think that it is good practice to put this binary artifact into the source repository. The image itself should come with a composer binary. I'm happy to help with this issue, if you want. :)
SamMousa commented 2017-02-27 10:08:42 +00:00 (Migrated from github.com)

@SKoschnicke I think the run command is just wrong. Composer is already included in the image.

https://github.com/ngineered/nginx-php-fpm/blob/master/Dockerfile#L177

@SKoschnicke I think the run command is just wrong. Composer is already included in the image. https://github.com/ngineered/nginx-php-fpm/blob/master/Dockerfile#L177
SKoschnicke commented 2017-02-27 11:15:49 +00:00 (Migrated from github.com)

I see. Good to know! The other problems are still valid, though.

I see. Good to know! The other problems are still valid, though.
richarvey commented 2017-03-16 16:49:13 +00:00 (Migrated from github.com)

@SKoschnicke sorry for the slow reply. I'd love some help with this. I've just merged in a better way to install composer but could you help with fixing the other issues please?

@SKoschnicke sorry for the slow reply. I'd love some help with this. I've just merged in a better way to install composer but could you help with fixing the other issues please?
SKoschnicke commented 2017-03-16 18:17:15 +00:00 (Migrated from github.com)

I'm currently ill but I think I can look into it on monday!

I'm currently ill but I think I can look into it on monday!
SKoschnicke commented 2017-03-27 07:45:17 +00:00 (Migrated from github.com)

In case nobody noticed: I fixed the issue in #130

In case nobody noticed: I fixed the issue in #130
richarvey commented 2017-04-03 10:18:44 +00:00 (Migrated from github.com)

@SKoschnicke Apologies been really busy will get this done asap

@SKoschnicke Apologies been really busy will get this done asap
loyd-cpi commented 2018-07-31 23:19:29 +00:00 (Migrated from github.com)

Hi Guys, I'm not really sure why this still happening to me. Is there an option to disable the autorun of composer install?

Hi Guys, I'm not really sure why this still happening to me. Is there an option to disable the autorun of composer install?
Sign in to join this conversation.
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
ric/nginx-php-fpm#95
No description provided.