Drop PHP 7.4 on master

[come-nc]

Starting with 26 we will no longer support PHP 7.4 Bump the minimum version and start removing workaround specific to PHP 7.4

• [ ] https://github.com/nextcloud/3rdparty/pull/1215

Parent issue: https://github.com/nextcloud/server/issues/35025

[come-nc]

ref https://github.com/nextcloud/server/issues/35025

[come-nc]

files_external tests are failing on PHPUnit >= 9, because we do:

		$this->assertContains('foopass, $json['authSchemes']);
		$this->assertContains('barauth', $json['authSchemes']);

While the values are actually present in the keys. Changing for:

		$this->assertContains('foopass', array_keys($json['authSchemes']));
		$this->assertContains('barauth', array_keys($json['authSchemes']));

[come-nc]

@icewind1991 Would you be able to publish icewind1991/samba-krb-test-apache with PHP 8 so that CI using that can run?

[come-nc]

There were 2 failures:
--
695 |  
696 | 1) Test\AppFramework\Controller\OCSControllerTest::testJSON
697 | Failed asserting that two strings are equal.
698 | --- Expected
699 | +++ Actual
700 | @@ @@
701 | -'{"ocs":{"meta":{"status":"ok","statuscode":100,"message":"OK","totalitems":"","itemsperpage":""},"data":{"test":"hi"}}}'
702 | +'{"ocs":{"meta":{"status":"ok","statuscode":"100","message":"OK","totalitems":"","itemsperpage":""},"data":{"test":"hi"}}}'
703 |  
704 | /drone/src/tests/lib/AppFramework/Controller/OCSControllerTest.php:105
705 |  
706 | 2) Test\Files\Type\DetectionTest::testDetectContent with data set #1 ('/data.tar.gz', 'application/x-gzip')
707 | Failed asserting that two strings are equal.
708 | --- Expected
709 | +++ Actual
710 | @@ @@
711 | -'application/x-gzip'
712 | +'application/octet-stream'
713 |  
714 | /drone/src/tests/lib/Files/Type/DetectionTest.php:88

[come-nc]

Rebased

[come-nc]

• [x] https://github.com/psalm/psalm-github-actions/issues/31

Even recent releases of psalm-github-actions are using php7.4. No activity since February. Should we fork and use our own image? Or is it possible to remove those CI steps and only use our already existing psalm step? Why do we have 3 psalm workflows?

[come-nc]

There were 2 failures:
--
695 |  
696 | 1) Test\AppFramework\Controller\OCSControllerTest::testJSON
697 | Failed asserting that two strings are equal.
698 | --- Expected
699 | +++ Actual
700 | @@ @@
701 | -'{"ocs":{"meta":{"status":"ok","statuscode":100,"message":"OK","totalitems":"","itemsperpage":""},"data":{"test":"hi"}}}'
702 | +'{"ocs":{"meta":{"status":"ok","statuscode":"100","message":"OK","totalitems":"","itemsperpage":""},"data":{"test":"hi"}}}'
703 |  
704 | /drone/src/tests/lib/AppFramework/Controller/OCSControllerTest.php:105
705 |  
706 | 2) Test\Files\Type\DetectionTest::testDetectContent with data set #1 ('/data.tar.gz', 'application/x-gzip')
707 | Failed asserting that two strings are equal.
708 | --- Expected
709 | +++ Actual
710 | @@ @@
711 | -'application/x-gzip'
712 | +'application/octet-stream'
713 |  
714 | /drone/src/tests/lib/Files/Type/DetectionTest.php:88

Fixed the first one, the second one comes from a change in PHP (or a dependency?) which returns mimetype application/gzip and not application/x-gzip anymore. See https://stackoverflow.com/questions/21870351/difference-between-x-gzip-and-gzip-for-content-encoding From what I understand we need to adapt https://github.com/nextcloud/server/blob/master/resources/config/mimetypemapping.dist.json and/or https://github.com/nextcloud/server/blob/master/resources/config/mimetypealiases.dist.json but it’s not clear to me how?

[come-nc]

I merged the psalm-* workflows into the static-code-analysis one and it seems to work :+1: Only problem is it appears as «5,022 new alerts including 53 errors» but these are not new, they are the same as before. Not sure how to tell github these are not new.

[come-nc]

Fixed the first one, the second one comes from a change in PHP (or a dependency?) which returns mimetype application/gzip and not application/x-gzip anymore. See https://stackoverflow.com/questions/21870351/difference-between-x-gzip-and-gzip-for-content-encoding From what I understand we need to adapt https://github.com/nextcloud/server/blob/master/resources/config/mimetypemapping.dist.json and/or https://github.com/nextcloud/server/blob/master/resources/config/mimetypealiases.dist.json but it’s not clear to me how?

This is annoying, I tried fixing that by having both gzip and x-gzip versions in mimetypemapping.dist.json but our code is not consistent between detectPath and detectContent, somehow one returns first value and the other one the second one. Looks like a bug to me but I do not understand much about our mimetype detection system.

[ChristophWurst]

Are there any parts of this PR that you could extract and submit separately?

[skjnldsv]

Rebased with latest actions changes. This PR should be a bit lighter now :)

[skjnldsv]

Considering the 891 files in from php cs, we can tackle that as a followup maybe! :see_no_evil:

[come-nc]

Are there any parts of this PR that you could extract and submit separately?

Not much if we want passing CI, there are all kinds of chicken&egg problems in here.

[come-nc]

I merged the psalm-* workflows into the static-code-analysis one and it seems to work +1 Only problem is it appears as «5,022 new alerts including 53 errors» but these are not new, they are the same as before. Not sure how to tell github these are not new.

@skjnldsv Part of this was lost by your split+rebase. I still have it locally.

What’s missing is merging .github/workflows/psalm-github.yml and .github/workflows/static-code-analysis.yml together to avoid running psalm twice and make sure our baseline is correctly used.

My version of the file is static-code-analysis.yml.txt

[come-nc]

Are there any parts of this PR that you could extract and submit separately?

https://github.com/nextcloud/server/pull/35943

[come-nc]

• [x] https://github.com/icewind1991/samba-krb-test/pull/1

[skjnldsv]

What’s missing is merging .github/workflows/psalm-github.yml and .github/workflows/static-code-analysis.yml together to avoid running psalm twice and make sure our baseline is correctly used.

Yes, it was on purpose, I assumed we went different ways. Was there any reason? It works well on master now :thinking: I merged the two psalm-github jobs already, we can make them all in the same file sure, but I figured it's cleaner to differentiate the baselines one than the one that are just here to send their data for github to show ?

[come-nc]

Yes, it was on purpose, I assumed we went different ways. Was there any reason? It works well on master now thinking I merged the two psalm-github jobs already, we can make them all in the same file sure, but I figured it's cleaner to differentiate the baselines one than the one that are just here to send their data for github to show ?

2 reasons for merging:

• Avoid running psalm twice (we are already running it in static-code-analysis, we can just upload the result without running it again in another runner)
• Avoid maintaining 2 configurations, errors which were added to baseline sometimes still had to be ignored by hand on the github version, some errors showed in github but not in the other one because the PHP extensions or versions were not the same, …

[come-nc]

Rebased

[come-nc]

• [x] Find a way to fix the integrity test

[nickvergessen]

Find a way to fix the integrity test

https://github.com/nextcloud/server/blob/6a1510f8ee48494a19dc1239fd2584dc2188e18d/tests/lib/IntegrityCheck/CheckerTest.php#L764-L769

So I did:

nano tests/lib/IntegrityCheck/CheckerTest.php
./autotest.sh sqlite tests/lib/IntegrityCheck/CheckerTest.php
sudo -u www-data php occ integrity:sign-core --privateKey=./tests/data/integritycheck/core.key --certificate=./tests/data/integritycheck/core.crt --path=./tests/data/integritycheck/mimetypeListModified
git add tests/data/integritycheck/mimetypeListModified/core/signature.json
git commit

then reverted the enabling line and now the test passes:

$ nano tests/lib/IntegrityCheck/CheckerTest.php
$ ./autotest.sh sqlite tests/lib/IntegrityCheck/CheckerTest.php
Using PHP executable /usr/bin/php
Using database oc_autotest
Setup environment for sqlite testing on local storage ...
0 Pfade vom Index aktualisiert
Installing ....
Nextcloud was successfully installed
Testing with sqlite ...
No coverage
/usr/bin/php /home/nickv/Tools/vendor/bin/phpunit --configuration phpunit-autotest.xml --log-junit autotest-results-sqlite.xml ../tests/lib/IntegrityCheck/CheckerTest.php 
PHPUnit 9.5.21 #StandWithUkraine

Runtime:       PHP 8.1.13
Configuration: phpunit-autotest.xml

...............................                                   31 / 31 (100%)

Time: 00:00.108, Memory: 30.00 MB

OK (31 tests, 89 assertions)

[come-nc]

• [x] https://github.com/nextcloud/docker-ci/pull/487

This should fix ldap integration jobs in drone.

[come-nc]

/rebase

[come-nc]

Drone CI passed :+1:

Remaining failures:

• Code scanning results / Psalm -> This comes from renaming the workflow, and will require a force merge. I think should be good on following PRs since it only shouts on new alerts. We should still look into fixing those taint alerts but that’s not for this PR
• Lint / php-cs -> This comes from a cs-fixer update which change a line on almost all PHP files in the repository. It was left out on purpose so that the PR changes are readable. It can be fixed by running cs:fix, either in a commit added after the reviews or in a separated PR afterwards.
• Samba Kerberos SSO / kerberos -> this is failing as well on other PR, should be fixed on its own in a followup PR

[come-nc]

This is now ready for review. Please do not merge as I will have to update the 3rdparty commit after merging overthere.

[nickvergessen]

Apps have been "fixed" in https://github.com/nextcloud/server/issues/36176

[DaphneMuller]

hello @come-nc Thank you so much for your work on this pull request! This ticket seems to have the tag 'pending documentation'. Is there any chance you could clarify what documentation is missing? Is this for admins or for app developers?

[MichaIng]

At least here the PHP requirements table needs to be adjusted: https://github.com/nextcloud/documentation/blob/master/admin_manual/installation/system_requirements.rst When touching it already, PHP 8.2 could be added as it is supported with Nextcloud 26.

There are further pages where PHP 7.4 is used as example, which should be updated to 8.1 or 8.2:

• https://github.com/nextcloud/documentation/blob/master/admin_manual/installation/nginx-root.conf.sample
• https://github.com/nextcloud/documentation/blob/master/admin_manual/installation/nginx-subdir.conf.sample
• https://github.com/nextcloud/documentation/blob/master/admin_manual/installation/example_centos.rst If CentOS 8 does not provide PHP 8.x, then the example should be completely reworked to use e.g. CentOS 9 or another recent RPM distro.

[come-nc]

hello @come-nc Thank you so much for your work on this pull request! This ticket seems to have the tag 'pending documentation'. Is there any chance you could clarify what documentation is missing? Is this for admins or for app developers?

https://github.com/nextcloud/documentation/pull/9650

[come-nc]

https://github.com/nextcloud/server/issues/34692#issuecomment-1438663490