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

Allow composer operations after starter project #312

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

Conversation

noahwsmith
Copy link
Contributor

As per #306, the current starter and starter_dev leave the codebase with permissions that do not allow further composer operations without surgery. I tweaked the end of the finalize-starter function to address this.

To replicate:

  • Checkout this branch
  • make clean then make starter_dev and then docker-compose exec drupal bash -c "composer update"

Results for me (success!):

Noahs-M2-MBP-4:isle-dc-2 noah$ docker-compose exec drupal bash
bash-5.1# composer update
Loading composer repositories with package information
Info from https://repo.packagist.org: #StandWithUkraine
Updating dependencies
Lock file operations: 1 install, 25 updates, 0 removals
  - Upgrading consolidation/annotated-command (4.5.6 => 4.6.1)
  - Upgrading consolidation/output-formatters (4.2.2 => 4.2.3)
  - Upgrading consolidation/site-alias (3.1.5 => 3.1.7)
  - Upgrading consolidation/site-process (4.2.0 => 4.2.1)
  - Upgrading doctrine/event-manager (1.1.2 => 1.2.0)
  - Locking drupal/ctools (4.0.1)
  - Upgrading drupal/field_group (3.3.0 => 3.4.0)
  - Upgrading drupal/search_api (1.27.0 => 1.28.0)
  - Upgrading islandora/islandora (2.4.3 => 2.5.0)
  - Upgrading islandora/openseadragon (2.0.2 => 2.0.3)
  - Upgrading nesbot/carbon (2.62.1 => 2.63.0)
  - Upgrading nikic/php-parser (v4.15.1 => v4.15.2)
  - Upgrading symfony/console (v4.4.45 => v4.4.48)
  - Upgrading symfony/http-foundation (v4.4.46 => v4.4.48)
  - Upgrading symfony/http-kernel (v4.4.46 => v4.4.48)
  - Upgrading symfony/polyfill-intl-grapheme (v1.26.0 => v1.27.0)
  - Upgrading symfony/polyfill-php72 (v1.26.0 => v1.27.0)
  - Upgrading symfony/polyfill-php73 (v1.26.0 => v1.27.0)
  - Upgrading symfony/property-access (v5.4.11 => v5.4.15)
  - Upgrading symfony/property-info (v5.4.11 => v5.4.15)
  - Upgrading symfony/security (v4.4.46 => v4.4.48)
  - Upgrading symfony/serializer (v4.4.45 => v4.4.47)
  - Upgrading symfony/string (v5.4.13 => v5.4.15)
  - Upgrading symfony/translation (v4.4.45 => v4.4.47)
  - Upgrading symfony/validator (v4.4.46 => v4.4.48)
  - Upgrading symfony/var-dumper (v5.4.13 => v5.4.14)
Writing lock file
Installing dependencies from lock file (including require-dev)
Package operations: 1 install, 25 updates, 0 removals
  - Downloading symfony/polyfill-php73 (v1.27.0)
  - Downloading symfony/console (v4.4.48)
  - Downloading consolidation/output-formatters (4.2.3)
  - Downloading doctrine/event-manager (1.2.0)
  - Downloading symfony/polyfill-php72 (v1.27.0)
  - Downloading symfony/validator (v4.4.48)
  - Downloading symfony/translation (v4.4.47)
  - Downloading symfony/serializer (v4.4.47)
  - Downloading symfony/http-foundation (v4.4.48)
  - Downloading symfony/var-dumper (v5.4.14)
  - Downloading symfony/http-kernel (v4.4.48)
  - Downloading drupal/field_group (3.4.0)
  - Downloading drupal/search_api (1.28.0)
  - Downloading consolidation/annotated-command (4.6.1)
  - Downloading nikic/php-parser (v4.15.2)
  - Downloading consolidation/site-alias (3.1.7)
  - Downloading consolidation/site-process (4.2.1)
  - Downloading nesbot/carbon (2.63.0)
  - Downloading symfony/polyfill-intl-grapheme (v1.27.0)
  - Downloading symfony/string (v5.4.15)
  - Downloading symfony/property-info (v5.4.15)
  - Downloading symfony/property-access (v5.4.15)
  - Downloading symfony/security (v4.4.48)
  - Downloading drupal/ctools (4.0.1)
  - Downloading islandora/islandora (2.5.0)
  - Downloading islandora/openseadragon (2.0.3)
  - Upgrading symfony/polyfill-php73 (v1.26.0 => v1.27.0): Extracting archive
  - Upgrading symfony/console (v4.4.45 => v4.4.48): Extracting archive
  - Upgrading consolidation/output-formatters (4.2.2 => 4.2.3): Extracting archive
  - Upgrading doctrine/event-manager (1.1.2 => 1.2.0): Extracting archive
  - Upgrading symfony/polyfill-php72 (v1.26.0 => v1.27.0): Extracting archive
  - Upgrading symfony/validator (v4.4.46 => v4.4.48): Extracting archive
  - Upgrading symfony/translation (v4.4.45 => v4.4.47): Extracting archive
  - Upgrading symfony/serializer (v4.4.45 => v4.4.47): Extracting archive
  - Upgrading symfony/http-foundation (v4.4.46 => v4.4.48): Extracting archive
  - Upgrading symfony/var-dumper (v5.4.13 => v5.4.14): Extracting archive
  - Upgrading symfony/http-kernel (v4.4.46 => v4.4.48): Extracting archive
  - Upgrading drupal/field_group (3.3.0 => 3.4.0): Extracting archive
  - Upgrading drupal/search_api (1.27.0 => 1.28.0): Extracting archive
  - Upgrading consolidation/annotated-command (4.5.6 => 4.6.1): Extracting archive
  - Upgrading nikic/php-parser (v4.15.1 => v4.15.2): Extracting archive
  - Upgrading consolidation/site-alias (3.1.5 => 3.1.7): Extracting archive
  - Upgrading consolidation/site-process (4.2.0 => 4.2.1): Extracting archive
  - Upgrading nesbot/carbon (2.62.1 => 2.63.0): Extracting archive
  - Upgrading symfony/polyfill-intl-grapheme (v1.26.0 => v1.27.0): Extracting archive
  - Upgrading symfony/string (v5.4.13 => v5.4.15): Extracting archive
  - Upgrading symfony/property-info (v5.4.11 => v5.4.15): Extracting archive
  - Upgrading symfony/property-access (v5.4.11 => v5.4.15): Extracting archive
  - Upgrading symfony/security (v4.4.46 => v4.4.48): Extracting archive
  - Installing drupal/ctools (4.0.1): Extracting archive
  - Upgrading islandora/islandora (2.4.3 => 2.5.0): Extracting archive
  - Upgrading islandora/openseadragon (2.0.2 => 2.0.3): Extracting archive
Package doctrine/reflection is abandoned, you should avoid using it. Use roave/better-reflection instead.
Package silex/silex is abandoned, you should avoid using it. Use symfony/flex instead.
Package symfony/debug is abandoned, you should avoid using it. Use symfony/error-handler instead.
Package webmozart/path-util is abandoned, you should avoid using it. Use symfony/filesystem instead.
Generating autoload files
68 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
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

#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.

@wgilling
Copy link
Contributor

wgilling commented May 3, 2023

This issue may be resolved when #329 is merged.

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.

4 participants