-
Notifications
You must be signed in to change notification settings - Fork 60
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 composer operations after starter project #312
base: development
Are you sure you want to change the base?
Conversation
#docker-compose exec -T drupal with-contenv bash -lc 'chown -R `id -u`:nginx /var/www/drupal' | ||
#docker-compose exec -T drupal with-contenv bash -lc 'drush migrate:rollback islandora_defaults_tags,islandora_tags' | ||
docker-compose exec -T drupal with-contenv bash -lc 'chown -R `id -u`:nginx /var/www/drupal' | ||
docker-compose exec drupal bash -c "chmod u+w web/sites/default/settings.php" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's a good idea to leave this writeable, as it goes against all drupal recommendations.
Every time I use composer
i also get errors, like this:
Scaffolding files for islandora/islandora-starter-site:
- NOTICE Modifying existing file at [web-root]/sites/default/settings.php. Examine the contents and ensure that it came out correctly.
- Append to [web-root]/sites/default/settings.php from assets/patches/default_settings.txt
[ErrorException]
file_put_contents(/var/www/drupal/web/sites/default/settings.php): failed to open stream: Operation not permitted
However, my updates always seem to succeed despite this error. I wonder if the first line of this change (the chown
) is enough to solve the fatal errors you were getting in #306 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Who are the authors of the new starter
and starter_dev
- maybe they could chime in about intentions and solutions here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After conversation on the tech call:
- it seems odd the permissions aree causing a problem on starter/starter dev but not local, since they're almost the same thing
- It would be fine to leave it writeable so long as that doesn't make it into production (or an open staging server). If needed, could be couched in warnings/documentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To paraphrase @DonRichards in the tech call: Might have somethign to do with the arguments (-T
?) present/absent in the docker-compose exec
command?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rosiel The issue seems to be specific to Macs, and it is only an issue on starter, because local doesn't try to append the patch to settings.php. From what I can tell, root on Macs can't write to a read only file, but on linux it can.
I think we would only need the file to be writable by root, but from my testing that gets changed every time you bring down the container. So I think this PR would fix it for the initial install, but then restarting containers would cause it to be not able to write again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed in tech call. Setting the file to writeable would be fragile (resetting when the containers are reloaded) and also Drupal will consider the writeable state to be an error, on the status pages.
This issue may be resolved when #329 is merged. |
As per #306, the current
starter
andstarter_dev
leave thecodebase
with permissions that do not allow further composer operations without surgery. I tweaked the end of thefinalize-starter
function to address this.To replicate:
make clean
thenmake starter_dev
and thendocker-compose exec drupal bash -c "composer update"
Results for me (success!):