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

Add support for TYPO3 v12 #4268

Merged
merged 3 commits into from
Oct 8, 2022
Merged

Conversation

gilbertsoft
Copy link
Member

@gilbertsoft gilbertsoft commented Oct 7, 2022

The Problem/Issue/Bug:

The new TYPO3 v12 major version has some breaking changes regarding the settings files.

How this PR Solves The Problem:

This patch introduces an automatic detection of the TYPO3 version 12 or higher and properly adapts the setting file paths and names.

To avoid the creation of the .gitignore in the wrong location, it is not written anymore before TYPO3 is installed or the version could be properly detected.

Manual Testing Instructions:

Follow the quick start guide to test this change. To test it for a previous version just add a version to the composer create command e.g.:

ddev composer create --no-install "typo3/cms-base-distribution:^11"

Automated Testing Overview:

Related Issue Link(s):

TYPO3/get.typo3.org#386

Release/Deployment notes:

@github-actions
Copy link

github-actions bot commented Oct 7, 2022

ddev start
ddev composer create "typo3/cms-base-distribution" --no-install
ddev composer install
ddev config --auto
Copy link
Member Author

Choose a reason for hiding this comment

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

Instead of ddev config --auto we also could recommend ddev restart to create the settings file. AFAIK this is a long standing issue anyway since the introduction of the --no-install option.

Copy link
Member

Choose a reason for hiding this comment

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

Why would we do a ddev config --auto when we just did the explicit ddev config above? It doesn't seem like this belongs at all.

Copy link
Member Author

Choose a reason for hiding this comment

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

Because the settings file isn't created without.

Copy link
Member Author

Choose a reason for hiding this comment

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

Other solution would be to create the files even with a simple ddev composer command.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've checked the sources again, at the moment the settings files are create with start, config and importdb. Because I now need TYPO3 properly to be installed for the detection of the version, the behavior has changed a bit and doesn't currently work without an additional line her.

Copy link
Member

Choose a reason for hiding this comment

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

Settings file is created on ddev start if the project type is set.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hope this should be clear now after your discussion yesterday and finally it's solved with #4273.

Copy link
Member

Choose a reason for hiding this comment

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

No, I never did understand, but glad about #4273.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I see now. A ddev restart after the ddev composer install would have worked and is probably better, especially wrt mutagen. But we can go with this as it is now.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I have already mentioned we can use both. So if you're happier with a restart, lets do a restart :)

@@ -62,7 +69,7 @@ func writeTypo3SettingsFile(app *DdevApp) error {
}

// The directory doesn't exist, create it with the appropriate permissions.
if err := os.Mkdir(dir, perms); err != nil {
if err := os.MkdirAll(dir, perms); err != nil {
Copy link
Member Author

Choose a reason for hiding this comment

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

This also fixes a long standing issue with older versions.

@rfay
Copy link
Member

rfay commented Oct 8, 2022

This one I'd dearly love to get into v1.21.2, which I hope to cut very soon, would have considered it today if this didn't come up. Thanks for the great work on this!

@gilbertsoft
Copy link
Member Author

Fixed the failing test today and all green again :)

@gilbertsoft gilbertsoft requested a review from rfay October 8, 2022 12:32
@rfay
Copy link
Member

rfay commented Oct 8, 2022

Congrats on tests, and thanks!

Copy link
Member

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

Thanks so much! Manually tested on v12 and v11, worked fine.

The two minor docs changes can be committed as a batch with [skip ci] in the commit title and it won't run a test.

docs/content/users/quickstart.md Outdated Show resolved Hide resolved
docs/content/users/quickstart.md Outdated Show resolved Hide resolved
gilbertsoft and others added 2 commits October 8, 2022 16:01
Co-authored-by: Randy Fay <randy@randyfay.com>
Co-authored-by: Randy Fay <randy@randyfay.com>
@rfay rfay merged commit bed20e5 into ddev:master Oct 8, 2022
@rfay rfay deleted the task/typo3-v12-support branch October 8, 2022 18:21
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

2 participants