Skip to content

[WIP] Drop PHP 7.2#38674

Closed
phil-davis wants to merge 8 commits into
masterfrom
try-drop-php-7.2
Closed

[WIP] Drop PHP 7.2#38674
phil-davis wants to merge 8 commits into
masterfrom
try-drop-php-7.2

Conversation

@phil-davis

@phil-davis phil-davis commented Apr 29, 2021

Copy link
Copy Markdown
Contributor

Description

This is to help debug the problem with newer releases of doctrine/dbal that fail CI. I will be able to choose specific doctrine/dbal releases to bisect exactly where the problem was introduced. (related to PRs #38604 and #38647 )

How Has This Been Tested?

CI

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE

@phil-davis phil-davis self-assigned this Apr 29, 2021
@update-docs

update-docs Bot commented Apr 29, 2021

Copy link
Copy Markdown

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@phil-davis

Copy link
Copy Markdown
Contributor Author

For the first commit, composer did these dependency bumps:

$ composer update
Loading composer repositories with package information
Updating dependencies
Lock file operations: 2 installs, 12 updates, 0 removals
  - Upgrading doctrine/dbal (2.10.4 => 2.13.1)
  - Locking doctrine/deprecations (v0.5.3)
  - Upgrading laminas/laminas-filter (2.9.4 => 2.11.0)
  - Upgrading laminas/laminas-inputfilter (2.10.1 => 2.12.0)
  - Upgrading laminas/laminas-servicemanager (3.5.2 => 3.6.4)
  - Upgrading laminas/laminas-stdlib (3.2.1 => 3.3.1)
  - Upgrading laminas/laminas-validator (2.13.5 => 2.14.4)
  - Upgrading laminas/laminas-zendframework-bridge (1.1.1 => 1.2.0)
  - Upgrading league/flysystem (1.0.70 => 1.1.3)
  - Locking league/mime-type-detection (1.7.0)
  - Upgrading phpunit/php-token-stream (3.1.2 => 4.0.4)
  - Upgrading roave/security-advisories (dev-master 0a55b3e => dev-master a9d356b)
  - Upgrading symfony/service-contracts (v1.1.9 => v2.4.0)
  - Upgrading symfony/translation-contracts (v1.1.10 => v2.4.0)

@mmattel

mmattel commented May 2, 2021

Copy link
Copy Markdown
Contributor

This is docs relevant when merged. Please file an issue close to merging

@phil-davis

phil-davis commented May 2, 2021

Copy link
Copy Markdown
Contributor Author

This will not be merged (at least not in the near future, probably more likely along with adding support for PHP 8.0)
I am using it to investigate what are the problems with newer releases of doctrine/dbal - and some of those only support PHP 7.3/7.4. So I need to remove PHP 7.2 from CI just for the investigation.

@phil-davis

phil-davis commented May 3, 2021

Copy link
Copy Markdown
Contributor Author

Summary:
With doctrine/dbal 2.11.0 up to 2.13.1 the same problems happen. In QueryBuilder.php prefixTableName the code puts "PREFIX" in front of the table name. Later in the code, when constructing SQL commands, that is supposed to be replaced with the value of system setting dbtableprefix. Usually that is the default value oc_

But somehow the replacement is not happening any more when running with the updated doctrine/dbal.

The first that that goes wrong is when installing oC10:

SQLSTATE[HY000]: General error: 1 no such table: *PREFIX*appconfig

And if we get past that, then lots of other code has a similar problem.

I have completely hacked it here (put in a hard-coded oc_ !) and adjusted some unit tests, and a lot of the unit tests pass, and the acceptance tests are passing. We definitely do not want this hack!

I will make an issue for someone to look into this: #38681

@phil-davis

Copy link
Copy Markdown
Contributor Author

PR #38647 is green and ready to merge. This "hack" PR can be closed.

@phil-davis phil-davis closed this May 4, 2021
@phil-davis phil-davis deleted the try-drop-php-7.2 branch May 4, 2021 11:08
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.

2 participants