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

Handle PDO driver extensions #253

Open
wants to merge 2 commits into
base: 1.41.x
Choose a base branch
from

Conversation

boesing
Copy link
Member

@boesing boesing commented Sep 26, 2024

Q A
Bugfix yes/no
BC Break no
New Feature yes

Description

Projects composer.json can require specific PDO drivers such as ext-pdo_mysql. These are passed to the CI container as pdo_mysql and are then passed as php<version>-pdo_mysql package to apt install. These packages do not exist and thus there will be an error if the appropriate extension(s) was/were not already installed/enabled. This patch handles the most common drivers mysql, pgsql and sqlite and ensure that these are installed/enabled along with the pdo extension.

fixes #252

Projects `composer.json` can require specific PDO drivers such as `ext-pdo_mysql`. These are passed to the CI container as `pdo_mysql` and are then passed as `php<version>-pdo_mysql` package to `apt install`. These packages do not exist and thus there will be an error if the appropriate extension(s) was/were not already installed/enabled.
This patch handles the most common drivers `mysql`, `pgsql` and `sqlite` and ensure that these are installed/enabled along with the `pdo` extension.

Signed-off-by: Maximilian Bösing <[email protected]>
@boesing boesing added this to the 1.41.0 milestone Sep 26, 2024
@boesing boesing requested review from Xerkus and gsteel September 26, 2024 16:57
@Xerkus
Copy link
Member

Xerkus commented Oct 1, 2024

Why not just install them by default? It is not like they are too large.

@boesing
Copy link
Member Author

boesing commented Oct 1, 2024

Why not install any extension by default then? 🤷🏼‍♂️
I do not mind having it installed by default, just want to have it available - so yes, that could be another solution.

Copy link
Member

@gsteel gsteel left a comment

Choose a reason for hiding this comment

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

LGTM 👍 - I also don't see any problem with installing these by default

@boesing
Copy link
Member Author

boesing commented Oct 4, 2024

for the sake of simplicity, I will change my PR to include these as default then.

The main idea of the container initially was that it is small but if we agree on having it containing a bunch of defaults, that would work for me.
IMHO, database stuff is not used that much with this container but thats a different topic.

@alexmerlin
Copy link
Member

@boesing Can you continue work on this one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PDO driver support
4 participants