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

[stable10] drop php 5.6 support #34698

Merged
merged 10 commits into from
Mar 12, 2019
Merged

[stable10] drop php 5.6 support #34698

merged 10 commits into from
Mar 12, 2019

Conversation

phil-davis
Copy link
Contributor

@phil-davis phil-davis commented Mar 6, 2019

Description

These are the easy things.

1a) bump in index.php and console.php

1b) adjust matrix entries in .drone.yml

2a) let composer find the latest things based on PHP 7.0
composer decided on :

composer update
Loading composer repositories with package information
Updating dependencies (including require-dev)
Package operations: 0 installs, 4 updates, 0 removals
  - Updating paragonie/random_compat (v2.0.18 => v9.99.99): Loading from cache
  - Updating doctrine/inflector (v1.1.0 => v1.2.0): Downloading (100%)         
  - Updating phpunit/php-token-stream (1.4.12 => 2.0.2): Loading from cache
  - Updating phpdocumentor/reflection-docblock (3.3.2 => 4.3.0): Loading from cache

When doing the composer bump, it complained that symfony requires PHP 7.0.8 minimum. I guess there was some PHP fix in 7.0.8 that symfony really wants. So make that the absolute minimum patch level specified in composer.json

  1. Make the corresponding bump to 7.0.8 in index.php and console.php

  2. But SLES12 SP4 delivers a PHP that reports version 7.0.7 but it really has been patched with the "good" 7.0.8 stuff. So in index.php and console.php allow a minimum reported PHP version 7.0.7

  3. Bump phpseclib/phpseclib (2.0.14 => 2.0.15) (a recent patch release while this PR is being developed)

6,7,8) cherry-pick backport of drone composer cache fixups (see backport PR #34717 )

  1. Run phan for PHP 7.0 in a separate drone job. An older version of phan is needed for PHP 7.0, so the composer step for that needs to also be running PHP 7.0.

  2. Add some smarts to Makefile for phpstan so that it does not try to ever composer install it when running PHP 7.0. phpstan has no PHP 7.0 support!

Related Issue

  • Fixes <issue_link>

Motivation and Context

See what test complain, if any.

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:

Open tasks:

  • Backport (if applicable set "backport-request" label and remove when the backport was done)

@PVince81
Copy link
Contributor

PVince81 commented Mar 6, 2019

note to self:

  • bump release server min PHP version when building before we merge this, else the dailies will not be built...

@phil-davis
Copy link
Contributor Author

CI passed, so this first minimal set of changes seems "a good thing" (tm)

@phil-davis
Copy link
Contributor Author

phil-davis commented Mar 6, 2019

Maybe also when merging this:

Copy link
Contributor

@patrickjahns patrickjahns left a comment

Choose a reason for hiding this comment

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

For now the minimum is 7.0.7 based on SLES12 SP4 requirement from customers.
Let's double check that and see if we can move to 7.0.8 . minimum or if we have to keep lower

@patrickjahns patrickjahns self-requested a review March 6, 2019 15:37
Copy link
Contributor

@patrickjahns patrickjahns left a comment

Choose a reason for hiding this comment

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

@phil-davis
Copy link
Contributor Author

phil-davis commented Mar 6, 2019

Please use php7.0.7 as baseline for SLES 12 SP4 ( see: https://scc.suse.com/packages?name=SUSE%20Linux%20Enterprise%20Server&version=12.4&arch=x86_64&query=php7&module= )

OK, that introduces a challenge:
Tell composer to find a set of dependencies to work with 7.0.7:

diff --git a/composer.json b/composer.json
index 3de8a248cd..d4d2535846 100644
--- a/composer.json
+++ b/composer.json
@@ -7,7 +7,7 @@
         "optimize-autoloader": true,
         "classmap-authoritative": false,
         "platform": {
-            "php": "5.6.33"
+            "php": "7.0.7"
         }
     },
     "autoload" : {
@@ -26,7 +26,7 @@
         "roave/security-advisories": "dev-master"
     },
     "require": {
-        "php": ">=5.6",
+        "php": ">=7.0.7",
         "doctrine/dbal": "^2.5",
         "phpseclib/phpseclib": "^2.0",
         "rackspace/php-opencloud": "v1.9.2",

Then ask composer to sort it out:

composer update
Loading composer repositories with package information
Updating dependencies (including require-dev)
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - symfony/translation v3.4.9 requires php ^5.5.9|>=7.0.8 -> your PHP version (7.2.15-1+ubuntu18.04.1+deb.sury.org+1) overridden by "config.platform.php" version (7.0.7) does not satisfy that requirement.
    - symfony/translation v3.4.8 requires php ^5.5.9|>=7.0.8 -> your PHP version (7.2.15-1+ubuntu18.04.1+deb.sury.org+1) overridden by "config.platform.php" version (7.0.7) does not satisfy that requirement.
    - symfony/translation v3.4.7 requires php ^5.5.9|>=7.0.8 -> your PHP version (7.2.15-1+ubuntu18.04.1+deb.sury.org+1) overridden by "config.platform.php" version (7.0.7) does not satisfy that requirement.
    - symfony/translation v3.4.6 requires php ^5.5.9|>=7.0.8 -> your PHP version (7.2.15-1+ubuntu18.04.1+deb.sury.org+1) overridden by "config.platform.php" version (7.0.7) does not satisfy that requirement.
    - symfony/translation v3.4.5 requires php ^5.5.9|>=7.0.8 -> your PHP version (7.2.15-1+ubuntu18.04.1+deb.sury.org+1) overridden by "config.platform.php" version (7.0.7) does not satisfy that requirement.
    - symfony/translation v3.4.4 requires php ^5.5.9|>=7.0.8 -> your PHP version (7.2.15-1+ubuntu18.04.1+deb.sury.org+1) overridden by "config.platform.php" version (7.0.7) does not satisfy that requirement.
    - symfony/translation v3.4.3 requires php ^5.5.9|>=7.0.8 -> your PHP version (7.2.15-1+ubuntu18.04.1+deb.sury.org+1) overridden by "config.platform.php" version (7.0.7) does not satisfy that requirement.
    - symfony/translation v3.4.23 requires php ^5.5.9|>=7.0.8 -> your PHP version (7.2.15-1+ubuntu18.04.1+deb.sury.org+1) overridden by "config.platform.php" version (7.0.7) does not satisfy that requirement.
    - symfony/translation v3.4.22 requires php ^5.5.9|>=7.0.8 -> your PHP version (7.2.15-1+ubuntu18.04.1+deb.sury.org+1) overridden by "config.platform.php" version (7.0.7) does not satisfy that requirement.
    - symfony/translation v3.4.21 requires php ^5.5.9|>=7.0.8 -> your PHP version (7.2.15-1+ubuntu18.04.1+deb.sury.org+1) overridden by "config.platform.php" version (7.0.7) does not satisfy that requirement.
    - symfony/translation v3.4.20 requires php ^5.5.9|>=7.0.8 -> your PHP version (7.2.15-1+ubuntu18.04.1+deb.sury.org+1) overridden by "config.platform.php" version (7.0.7) does not satisfy that requirement.
    - symfony/translation v3.4.2 requires php ^5.5.9|>=7.0.8 -> your PHP version (7.2.15-1+ubuntu18.04.1+deb.sury.org+1) overridden by "config.platform.php" version (7.0.7) does not satisfy that requirement.
    - symfony/translation v3.4.19 requires php ^5.5.9|>=7.0.8 -> your PHP version (7.2.15-1+ubuntu18.04.1+deb.sury.org+1) overridden by "config.platform.php" version (7.0.7) does not satisfy that requirement.
    - symfony/translation v3.4.18 requires php ^5.5.9|>=7.0.8 -> your PHP version (7.2.15-1+ubuntu18.04.1+deb.sury.org+1) overridden by "config.platform.php" version (7.0.7) does not satisfy that requirement.
    - symfony/translation v3.4.17 requires php ^5.5.9|>=7.0.8 -> your PHP version (7.2.15-1+ubuntu18.04.1+deb.sury.org+1) overridden by "config.platform.php" version (7.0.7) does not satisfy that requirement.
    - symfony/translation v3.4.16 requires php ^5.5.9|>=7.0.8 -> your PHP version (7.2.15-1+ubuntu18.04.1+deb.sury.org+1) overridden by "config.platform.php" version (7.0.7) does not satisfy that requirement.
    - symfony/translation v3.4.15 requires php ^5.5.9|>=7.0.8 -> your PHP version (7.2.15-1+ubuntu18.04.1+deb.sury.org+1) overridden by "config.platform.php" version (7.0.7) does not satisfy that requirement.
    - symfony/translation v3.4.14 requires php ^5.5.9|>=7.0.8 -> your PHP version (7.2.15-1+ubuntu18.04.1+deb.sury.org+1) overridden by "config.platform.php" version (7.0.7) does not satisfy that requirement.
    - symfony/translation v3.4.13 requires php ^5.5.9|>=7.0.8 -> your PHP version (7.2.15-1+ubuntu18.04.1+deb.sury.org+1) overridden by "config.platform.php" version (7.0.7) does not satisfy that requirement.
    - symfony/translation v3.4.12 requires php ^5.5.9|>=7.0.8 -> your PHP version (7.2.15-1+ubuntu18.04.1+deb.sury.org+1) overridden by "config.platform.php" version (7.0.7) does not satisfy that requirement.
    - symfony/translation v3.4.11 requires php ^5.5.9|>=7.0.8 -> your PHP version (7.2.15-1+ubuntu18.04.1+deb.sury.org+1) overridden by "config.platform.php" version (7.0.7) does not satisfy that requirement.
    - symfony/translation v3.4.10 requires php ^5.5.9|>=7.0.8 -> your PHP version (7.2.15-1+ubuntu18.04.1+deb.sury.org+1) overridden by "config.platform.php" version (7.0.7) does not satisfy that requirement.
    - symfony/translation v3.4.1 requires php ^5.5.9|>=7.0.8 -> your PHP version (7.2.15-1+ubuntu18.04.1+deb.sury.org+1) overridden by "config.platform.php" version (7.0.7) does not satisfy that requirement.
    - symfony/translation v3.4.0 requires php ^5.5.9|>=7.0.8 -> your PHP version (7.2.15-1+ubuntu18.04.1+deb.sury.org+1) overridden by "config.platform.php" version (7.0.7) does not satisfy that requirement.
    - symfony/translation v3.4.23 requires php ^5.5.9|>=7.0.8 -> your PHP version (7.2.15-1+ubuntu18.04.1+deb.sury.org+1) overridden by "config.platform.php" version (7.0.7) does not satisfy that requirement.
    - Installation request for symfony/translation ^3.4 -> satisfiable by symfony/translation[v3.4.0, v3.4.1, v3.4.10, v3.4.11, v3.4.12, v3.4.13, v3.4.14, v3.4.15, v3.4.16, v3.4.17, v3.4.18, v3.4.19, v3.4.2, v3.4.20, v3.4.21, v3.4.22, v3.4.23, v3.4.3, v3.4.4, v3.4.5, v3.4.6, v3.4.7, v3.4.8, v3.4.9].

So symfony/translation 3.4.* works on PHP 5.6.* and on PHP 7.0.8+. For whatever reason, it claims to not work (properly) on PHP 7.0.7.

At the moment in stable10 we are selecting a set of dependencies that suit PHP 5.6.33 and that is putting some symfony/translation 3.4.* into composer.lock If a site really does run exactly PHP 7.0.7 then there is going to be some bug in PHP 7.0.7 that is fixed in PHP 7.0.8 and symfony/translation wants the benefit of that fix in PHP 7.0.8 - something from this long list? http://php.net/ChangeLog-7.php#7.0.8

I can back down to symfony/translation ^3.3 but then that loads other older symfony 3.3.* components, which are older than what we are already running now with stable10 and PHP 5.6. And those will have more unpatched bugs, and maybe will not work properly with PHP 7.0.7 anyway? (depending on if their dependency requirements ever got updated)

@patrickjahns
Copy link
Contributor

Can we find out what the issue was and double check if that code path oft the library is actually used in ownCloud.

@phil-davis
Copy link
Contributor Author

SP4 was just released in december: https://www.suse.com/releasenotes/x86_64/SUSE-SLES/12-SP4/ ...

https://forums.suse.com/showthread.php?12384-PHP-7-version

SUSE keep reporting that PHP 7.0.7 version number, but actually they backport security patches from 7.0.8, 7.0.9... and internally have versions like 7.0.7-50.38.2 - so in ownCloud we cannot rely on the patch level reported at runtime, because a SUSE system will say it is running 7.0.7 but actually it is patched beyond that!

Maybe we keep the composer.json 7.0.8 because we have to for it to find suitable dependencies. But put 7.0.7 as the minimum in console.php and index.php so that SLES12 SP4 sites to do not get locked out. Side-effect is that some other installation could run the "real" PHP 7.0.7 and maybe have an issue - but really there should be no sites still running "real" PHP 7.0.7

@patrickjahns
Copy link
Contributor

patrickjahns commented Mar 6, 2019

Taken from your reference

I ask because the version numbers that you may expect from
upstream do not exactly align forever within SLE because SUSE tries to
maintain a stable base, meaning they will backport fixes from 7.0.8,
7.1.x, and beyond back into 7.0.7. You can see these fixes either on
SUSE's security pages, or else in package changelogs themselves.

So we have no way of knowing if the fix required for translations is there?

@phil-davis
Copy link
Contributor Author

The reason for the symfony PHP 7.0.8 minimum requirement is in PR symfony/symfony#23703 which refers to PHP bug https://bugs.php.net/bug.php?id=72229
So somehow we search SUSE changelogs somewhere to confirm if that PHP bug has been patched back into the SUSE release of PHP 7.0.7-*

@patrickjahns
Copy link
Contributor

patrickjahns commented Mar 6, 2019

So far I could only see that CVEs are mentioned in changelogs - though the bug doesn't have a CVE assigned.

@patrickjahns
Copy link
Contributor

@crrodriguez has found the patch to be included in SLES12 SP4

https://build.opensuse.org/package/view_file/SUSE:SLE-12-SP4:Update/php7/php7-fix-wrong-reference-serialize.patch?expand=1

Let's go ahead with your suggestion of using 7.0.8 in composer - but setting index.php and console.php to 7.0.7

Any objections @PVince81 ?

@codecov
Copy link

codecov bot commented Mar 7, 2019

Codecov Report

Merging #34698 into stable10 will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff             @@
##             stable10   #34698   +/-   ##
===========================================
  Coverage          64%      64%           
  Complexity      19247    19247           
===========================================
  Files            1276     1276           
  Lines           75801    75801           
  Branches         1291     1291           
===========================================
  Hits            48515    48515           
  Misses          26907    26907           
  Partials          379      379
Flag Coverage Δ Complexity Δ
#javascript 53.22% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 65.15% <0%> (ø) 19247 <0> (ø) ⬇️
Impacted Files Coverage Δ Complexity Δ
index.php 0% <0%> (ø) 0 <0> (ø) ⬇️
console.php 0% <0%> (ø) 0 <0> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1e7165e...fe73120. Read the comment docs.

@codecov
Copy link

codecov bot commented Mar 7, 2019

Codecov Report

Merging #34698 into stable10 will increase coverage by 0.14%.
The diff coverage is 0%.

Impacted file tree graph

@@              Coverage Diff               @@
##             stable10   #34698      +/-   ##
==============================================
+ Coverage       64.04%   64.19%   +0.14%     
- Complexity      19258    19887     +629     
==============================================
  Files            1278     1278              
  Lines           75838    76245     +407     
  Branches         1291     1291              
==============================================
+ Hits            48572    48947     +375     
- Misses          26887    26919      +32     
  Partials          379      379
Flag Coverage Δ Complexity Δ
#javascript 53.22% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 65.35% <0%> (+0.15%) 19887 <0> (+629) ⬆️
Impacted Files Coverage Δ Complexity Δ
console.php 0% <0%> (ø) 0 <0> (ø) ⬇️
index.php 0% <0%> (ø) 0 <0> (ø) ⬇️
core/Command/App/CheckCode.php 34.31% <0%> (-2.77%) 43% <0%> (+22%)
lib/private/Repair/RepairUnmergedShares.php 30% <0%> (-2.53%) 41% <0%> (+1%)
lib/private/App/CodeChecker/CodeChecker.php 41.66% <0%> (-1.2%) 12% <0%> (+1%)
apps/dav/lib/CalDAV/Publishing/PublishPlugin.php 63.63% <0%> (-1.18%) 20% <0%> (+2%)
...e/Files/External/Service/GlobalStoragesService.php 89.69% <0%> (-0.94%) 18% <0%> (+1%)
lib/private/Files/Cache/Scanner.php 86.6% <0%> (-0.42%) 115% <0%> (+1%)
lib/private/User/Manager.php 81.29% <0%> (-0.41%) 56% <0%> (+2%)
apps/files/lib/Command/Scan.php 71.47% <0%> (-0.37%) 74% <0%> (+12%)
... and 90 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6ca9f4e...1fc4ef3. Read the comment docs.

@phil-davis
Copy link
Contributor Author

Commit added to back down the tested PHP version from 7.0.8 to 7.0.7
fe73120
As discussed above, composer.json still expects at least PHP 7.0.8, but you can login on a PHP 7.0.7 system because it might be running SLES12 SP4 which reports a "fake" PHP 7.0.7

@phil-davis phil-davis force-pushed the stable10-drop-php-5.6 branch 3 times, most recently from eda900a to d2a59c0 Compare March 8, 2019 11:38
@PVince81 PVince81 added this to the QA milestone Apr 12, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Apr 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants