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.

Sign up for GitHub

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

Jump to bottom

disable upgrades to 26 for 32-bit #34908

Merged
merged 1 commit into from
Nov 2, 2022
Merged

disable upgrades to 26 for 32-bit #34908

merged 1 commit into from
Nov 2, 2022

Conversation

szaimen
Copy link
Contributor

@szaimen szaimen commented Nov 1, 2022

Signed-off-by: szaimen szaimen@e.mail.de

@szaimen
disable upgrades to 26 for 32-bit
faa747e 
Signed-off-by: szaimen <szaimen@e.mail.de>
@szaimen szaimen added the 2. developing Work in progress label Nov 1, 2022
@szaimen szaimen added this to the Nextcloud 26 milestone Nov 1, 2022
@szaimen
Copy link
Contributor Author

szaimen commented Nov 1, 2022

/backport to stable25

@szaimen szaimen marked this pull request as ready for review November 1, 2022 14:13
@szaimen szaimen added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Nov 1, 2022
@szaimen szaimen requested review from a team, PVince81, ArtificialOwl and skjnldsv and removed request for a team November 1, 2022 14:14
@skjnldsv skjnldsv added the pending documentation This pull request needs an associated documentation update label Nov 2, 2022
@szaimen
Copy link
Contributor Author

szaimen commented Nov 2, 2022

Documentation update is already there: nextcloud/documentation#9274

@szaimen szaimen disabled auto-merge November 2, 2022 10:33
@szaimen szaimen merged commit 8655297 into master Nov 2, 2022
@szaimen szaimen deleted the enh/noid/disable-26-updates-32bit branch November 2, 2022 10:33
@szaimen szaimen removed the pending documentation This pull request needs an associated documentation update label Nov 2, 2022
@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Nov 2, 2022
Merged
@szaimen szaimen mentioned this pull request Nov 3, 2022
Closed
@nickvergessen
Copy link
Member

This does not block the updater (updater/updater.phar from release bundle).
I think we should have implemented it similarly to the PHP version check directly on the updater server (or should do that additionally)

@szaimen
Copy link
Contributor Author

szaimen commented Nov 3, 2022

You mean something like this?
nextcloud/updater#446

@szaimen
Copy link
Contributor Author

szaimen commented Nov 3, 2022

Reviews are welcome :)

@nickvergessen
Copy link
Member

nickvergessen commented Nov 4, 2022 •
edited
Loading

No, more like https://github.com/nextcloud/updater_server/blob/master/config/config.php#L159-L190 (note the minPHPVersion)
So that in 32 bit you get back the last working version instead of a newer one together with a "please replace your hardware" message which comes with a lot of effort.

nickvergessen
nickvergessen reviewed Nov 4, 2022
View reviewed changes
lib/private/Updater.php
@@ -138,6 +139,9 @@ public function upgrade(): bool {

$success = true;
try {
if (PHP_INT_SIZE < 8 && Semver::satisfies($currentVersion, '> 25')) {
throw new HintException('You are running a 32-bit PHP version. Cannot upgrade to Nextcloud 26 and higher. Please switch to 64-bit PHP.');
Copy link
Member

Choose a reason for hiding this comment

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

Also this is not really "32-bit PHP"
In https://github.com/nextcloud/updater/pull/446/files#diff-7413d6453f901e939bbd840c8f0d1c7b20c2ca0e7f71741e4e07c6cf036f16c0R171 the wording is a bit better with "32-bit system"

@szaimen
Copy link
Contributor Author

szaimen commented Nov 30, 2022

No, more like https://github.com/nextcloud/updater_server/blob/master/config/config.php#L159-L190 (note the minPHPVersion) So that in 32 bit you get back the last working version instead of a newer one together with a "please replace your hardware" message which comes with a lot of effort.

Could you give me some more hints what would need to be done to implement this? I am honestly a bit scared of the updater_server code...

@PVince81 PVince81 mentioned this pull request Jan 10, 2023
Closed
20 tasks
@come-nc come-nc mentioned this pull request Feb 7, 2023
Merged
4 tasks
blizzz added a commit that referenced this pull request Feb 7, 2023
@blizzz
Merge pull request #36588 from nextcloud/revert/34908
6077003 
Revert #34908 to allow 32bit setups to upgrade to 26
come-nc added a commit that referenced this pull request Feb 9, 2023
@come-nc
Merge pull request #36593 from nextcloud/backport/36588/stable25
a8097b5 
[stable25] Revert #34908 to allow 32bit setups to upgrade to 26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nickvergessen nickvergessen nickvergessen left review comments

@artonge artonge artonge approved these changes

@skjnldsv skjnldsv skjnldsv approved these changes

@PVince81 PVince81 Awaiting requested review from PVince81

@ArtificialOwl ArtificialOwl Awaiting requested review from ArtificialOwl

@blizzz blizzz Awaiting requested review from blizzz

@CarlSchwan CarlSchwan Awaiting requested review from CarlSchwan

Assignees
No one assigned
Labels
3. to review Waiting for reviews
Projects
None yet
Milestone
Nextcloud 26
Development

Successfully merging this pull request may close these issues.
4 participants
@szaimen @nickvergessen @artonge @skjnldsv