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

Update README.md #149

Closed
wants to merge 1 commit into from
Closed

Update README.md #149

wants to merge 1 commit into from

Conversation

joshribakoff
Copy link
Contributor

The web installer doesn't run without first setting the file permissions

The web installer doesn't run without first setting the file permissions

``` bash
$ cd sylius
$ vi app/config/parameters.yml # And put your values!
$ chmod -R 777 app/cache
$ chmod -R 777 app/logs
Copy link
Contributor

Choose a reason for hiding this comment

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

$ sudo setfacl -R -m u:www-data:rwX -m u:`whoami`:rwX app/cache app/logs
$ sudo setfacl -dR -m u:www-data:rwx -m u:`whoami`:rwx app/cache app/logs

http://symfony.com/doc/current/book/installation.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't understand why you're linking the symfony documentation. Are you implying my PR is unwanted, and someone installing Sylius should just intuitively know to go read the docs for all your dependencies? At least link to the 3rd party docs in that case. Just trying to help, because your installation has a few steps, some of which aren't documented.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I was writing from the phone and was in a hurry, so wanted to respond promptly but keep it short. Now I see it is not clear. I will try to explain.

I just think we should replace:

$ chmod -R 777 app/cache
$ chmod -R 777 app/logs

with

$ sudo setfacl -R -m u:www-data:rwX -m u:`whoami`:rwX app/cache app/logs
$ sudo setfacl -dR -m u:www-data:rwx -m u:`whoami`:rwx app/cache app/logs

And in Setting up Permissions section it is described how to set this permissions properly.

Copy link

Choose a reason for hiding this comment

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

is setfacl possible on macs?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure, it should be

$ sudo chmod +a "www-data allow delete,write,append,file_inherit,directory_inherit" app/cache app/logs
$ sudo chmod +a "`whoami` allow delete,write,append,file_inherit,directory_inherit" app/cache app/logs

or

$ sudo setfacl -R -m u:www-data:rwX -m u:`whoami`:rwX app/cache app/logs
$ sudo setfacl -dR -m u:www-data:rwx -m u:`whoami`:rwx app/cache app/logs

or both.

Copy link

Choose a reason for hiding this comment

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

777 can be used in production IF the umask() is set correctly as is commented in web/app*.php

Copy link

Choose a reason for hiding this comment

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

perhaps linking to the symfony instructions would be the best bet.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can just put a link to the symfony installation doc... If you run all sf command with the user apache it will work.

It will not work because if you run a sf command with a local (different than apache), the log and cache files will get the rights defined with umask, no? By default, I think it is 755, no? those generated files get the user and the group of the user who run the command. For example local user clear the cache, all cache files will have the local user as owner (and his default). After that, apache will can not overwrite cache files and it will return you a exception!

I hope you understand me...

Copy link
Contributor

Choose a reason for hiding this comment

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

Exactly. CLI user is different then apache and you will have ownership
problems. That is another advantage of sf doc approach comparing to 777.
On 10 Jun 2013 21:51, "Arnaud Langlade" notifications@github.com wrote:

In README.md:

 $ cd sylius
 $ vi app/config/parameters.yml # And put your values!
+$ chmod -R 777 app/cache
+$ chmod -R 777 app/logs

Indeed, jjanvier has right. We can not put chmod -R 777 in the
documentation, it is not very safety... We can just put a link to the
symfony installation doc...

Finally I think that chmod -R 777 will not work because if you run a sf
command with a local (different than apache), the log and cache files will
get the rights defined with umask property, no? By default, I think it is
755, no? those generated files get the user and the group of the user who
run the command. For example local user clear the cache, all cache files
will have the local user as owner (and his default). After that, apache
will can not overwrite cache files and it will return you a exception!

I hope you understand me...


Reply to this email directly or view it on GitHubhttps://github.com//pull/149/files#r4619845
.

Copy link
Contributor

Choose a reason for hiding this comment

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

With installer you get hints how to fix permissions, so the right place to change this is installer bundle.

selection_087

@ghost
Copy link

ghost commented Jun 13, 2013

@joshribakoff : in the future please use a more descriptive PR title than what github gives you by default

@arnolanglade
Copy link
Contributor

We can remove vi app/config/parameters.yml, now the installer generates the paramaters.yml.

@stloyd
Copy link
Contributor

stloyd commented Sep 26, 2013

Closing this as the README file was changed already and some cases are covered by SyliusInstallerBundle.

@stloyd stloyd closed this Sep 26, 2013
pamil pushed a commit to pamil/Sylius that referenced this pull request Mar 21, 2016
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.

None yet

5 participants