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.

Sign up for GitHub

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

Jump to bottom

Allow composer operations after starter project #312

Open
wants to merge 1 commit into
base: development
Choose a base branch
from
Open

Allow composer operations after starter project #312

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,6 @@ starter-finalize:
docker-compose exec -T drupal with-contenv bash -lc "drush -l $(SITE) user:role:add fedoraadmin admin"
MIGRATE_IMPORT_USER_OPTION=--userid=1 $(MAKE) hydrate
docker-compose exec -T drupal with-contenv bash -lc 'drush -l $(SITE) migrate:import --userid=1 islandora_fits_tags'
#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"
Copy link
Contributor

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 ?

Copy link
Contributor Author

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?

Copy link
Contributor

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

Copy link
Contributor

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?

Copy link
Contributor

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.

Copy link
Contributor

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.

$(MAKE) login