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

Prepare for DDEV 1.23 and improve initial config. #23

Closed
wants to merge 2 commits into from

Conversation

simesy
Copy link
Contributor

@simesy simesy commented Apr 8, 2024

This PR prepares the plugin for ddev 1.23 and also is a little more explicit about its config including setting some initial config to make sure ddev inits correctly.

Please note that if you want to reinstall Drupal. there is a known issue with the sites/default being hardened and it's not obvious the user how to reset drupal. A future PR will fix this.

@rpkoller has more extensive testing and he should weigh in on the PR to make sure he's happy.

Anyone testing should note to vary the README by cloning my fork inside the drupal repo, and then doing ddev get ddev-drupal-core-dev to use the clone as the plugin source.

Tested against 1.22.7 and 1.23-beta1

  1. Clone drupal, cd drupal
  2. git clone git@github.com:simesy/ddev-drupal-core-dev.git (should be main branch)
  3. ddev config --omit-containers=db --disable-settings-management
  4. ddev start
  5. ddev get ddev-drupal-core-dev
  6. ddev restart
  7. ddev composer install

@simesy
Copy link
Contributor Author

simesy commented Apr 8, 2024

@rpkoller says a remaining issue is restarting ddev for yarn version (resolved)

@rfay
Copy link
Contributor

rfay commented Apr 8, 2024

One thought: If you want to make it require DDEV v1.23, which would make things simpler, config.*.yaml could require DDEV v1.23+ by adding

ddev_version_constraint >= v1.23.0-alpha1

https://ddev.readthedocs.io/en/stable/users/configuration/config/#ddev_version_constraint

web-build/Dockerfile Outdated Show resolved Hide resolved
@rpkoller
Copy link

rpkoller commented Apr 8, 2024

One thought: If you want to make it require DDEV v1.23, which would make things simpler, config.*.yaml could require DDEV v1.23+ by adding

it would be less jumping through hoops by simply requiring 1.23+ and it would also save users from the annoying redownload of yarn in 1.22.7

        Corepack is about to download https://registry.yarnpkg.com/yarn/-/yarn-1.22.22.tgz.

        Do you want to continue? [Y/n]

@rfay
Copy link
Contributor

rfay commented Apr 8, 2024

it would be less jumping through hoops by simply requiring 1.23+ and it would also save users from the annoying redownload of yarn in 1.22.7

If I'm not mistaken, it will only save users from this the second and following times. The reason is that DDEV v1.23+ sets the COREPACK_HOME to a persistent volume, so yarn is there there the next time it's looked for, unless, of course, the yarn version has changed and a new one has to be fetched.

@rpkoller
Copy link

rpkoller commented Apr 8, 2024

yep in my tests on 1.22.7 you had to redownload yarn after every restart of the project.

@simesy
Copy link
Contributor Author

simesy commented Apr 9, 2024

Either option suits me. If this was a plugin that people used for their paid work then I would question forcing them to upgrade (in case there was a reason they were pinned to an older DDEV version). Any performance improvements can be follow up.

Happy to act on instruction.

@pcambra
Copy link

pcambra commented Apr 26, 2024

Just adding the woraround to get Drupal 11 working from @stasadev on ddev/ddev#6110 (comment)

  1. Create a file .ddev/web-build/Dockerfile.sqlite
  2. The file should have this content:
ARG TARGETPLATFORM
RUN SQLITE_VERSION=3.45.1 && \
    mkdir -p /tmp/sqlite3 && \
    wget -O /tmp/sqlite3/sqlite3.deb https://ftp.debian.org/debian/pool/main/s/sqlite3/sqlite3_${SQLITE_VERSION}-1_${TARGETPLATFORM##linux/}.deb && \
    wget -O /tmp/sqlite3/libsqlite3.deb https://ftp.debian.org/debian/pool/main/s/sqlite3/libsqlite3-0_${SQLITE_VERSION}-1_${TARGETPLATFORM##linux/}.deb && \
    dpkg -i /tmp/sqlite3/*.deb && \
    rm -rf /tmp/sqlite3
  1. ddev restart

web-build/Dockerfile Outdated Show resolved Hide resolved
Copy link
Contributor

@rfay rfay left a comment

Choose a reason for hiding this comment

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

I think it's really important to get this over the finish line ASAP so we'll have it mature and released by Drupalcon Portland a week from now (May 5).

Tested with:

ddev get https://github.com/simesy/ddev-drupal-core-dev/tarball/main

It does require @rpkoller's .ddev/Dockerfile.ddev-drupal-core-dev changes to get Sqlite3 3.45

web-build/Dockerfile Show resolved Hide resolved
web-build/Dockerfile Outdated Show resolved Hide resolved
config.ddev-drupal-core-dev.yaml Outdated Show resolved Hide resolved
web-build/Dockerfile Outdated Show resolved Hide resolved
config.ddev-drupal-core-dev.yaml Show resolved Hide resolved
config.ddev-drupal-core-dev.yaml Show resolved Hide resolved
install.yaml Outdated Show resolved Hide resolved
@rfay
Copy link
Contributor

rfay commented Apr 27, 2024

@justafish if you could also run the workflow it would be great. Or give perms to me to do it.

I'm assuming this add-on has a bright future, right? Do you want to move it to official in the ddev org? You'd remain maintainer.

Copy link
Contributor

@rfay rfay left a comment

Choose a reason for hiding this comment

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

It looks to me like this is ready to go. @justafish if you could run tests to make sure it's OK, and pull. Next steps we could make sure it works consistently with one database provider and test and then do a release.

Copy link
Contributor

@rfay rfay left a comment

Choose a reason for hiding this comment

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

@justafish if we can get this in then I'll add to it support for drush and automatic setup of the proper SIMPLETEST_DB info in the phpunit.xml. I'll open an issue about drush explaining how I'd like to do it.

@simesy
Copy link
Contributor Author

simesy commented May 1, 2024

Closed in favour of #30 as i changed the base branch in the forked repo to upstream.

@simesy simesy closed this May 1, 2024
justafish pushed a commit that referenced this pull request May 23, 2024
* DDEV 1.23 compatiblity, Drupal core yarn/sqlite compatibility. Docs.

* Need to use sqlite now b/c no mysql. From /pull/27/files.

* Remove install of sqlite3 and make dependent on DDEV v1.23.1

* Run tests now that sqlite 3.45 in DDEV release

* Dockerfile is gone, don't try to install it

* add back in omit_containers, can remove farther down the line

* Remove --verbose from phpunit command, not supported in phpunit 10

---------

Co-authored-by: Simon Hobbs <simon@hobbs.id.au>
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

4 participants