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

Upgrader - various issues learned via CLI #6036

Merged
merged 12 commits into from Apr 9, 2020

Conversation

sbulen
Copy link
Contributor

@sbulen sbulen commented Mar 5, 2020

Fixes #5981 (in conjunction with #5986 )
Fixes #6001

CLI, especially with 1.x, was generating scores of errors.

Changes:

  • The basic problem was that the '@' operator still invokes the error handler, and they get logged, even though they are OK. Not a big deal, other than this was masking some REAL issues. Note that although the @ operator still calls the error routine, it temporarily sets the error level to 0, so it is easy to test in the error handler. So, the real errors were...
  • Some true undefined errors were buried in there - and not visible via browser upgrades.
  • Fixed the utf8 template issue.
  • cmdstep0 is the cli version of WelcomeLogin (without a template) & needed to deal with custom_avatar

A subset of these changes are needed in 2.0 as well.

Testing completed...

Signed by Shawn Bulen, bulens@pacbell.net
Signed by Shawn Bulen, bulens@pacbell.net
Signed by Shawn Bulen, bulens@pacbell.net
Signed by Shawn Bulen, bulens@pacbell.net
Signed by Shawn Bulen, bulens@pacbell.net
Signed by Shawn Bulen, bulens@pacbell.net
@sbulen sbulen added the Upgrader label Mar 5, 2020
other/upgrade.php Outdated Show resolved Hide resolved
Signed by Shawn Bulen, bulens@pacbell.net
Signed by Shawn Bulen, bulens@pacbell.net
@sbulen
Copy link
Contributor Author

sbulen commented Mar 16, 2020

That last commit addressed undefined offset issues experienced with most upgrades:
yabb-1

@sbulen
Copy link
Contributor Author

sbulen commented Mar 16, 2020

The last issue I have found during testing is... Interesting...

Via CLI, I am seeing a class of undefined errors that are pretty big & cause functionality gaps - but there is no corresponding gap when the upgrade is run via the browser.

The difference is that the browser "comes up for air" at various points & restarts. And reloads $modSettings... CLI never reloads $modSettings, so the only values it knows are the ones that were there at the start of the upgrade. The loss of functionality is thus worse the further back you go.

I have some thoughts to address. If easily fixed, we're good. If not, I think guidance needs to be that CLI isn't really appropriate for meaningful upgrades across versions.

Some examples:
Old-upgr-1
Old-upgr-2

Signed by Shawn Bulen, bulens@pacbell.net
Signed by Shawn Bulen, bulens@pacbell.net
@sbulen
Copy link
Contributor Author

sbulen commented Mar 20, 2020

Still testing this. Lots of permutations...

Some variations get this at the very end - looking into it:
yabb

I suspect this was brought about by the auth_secret changes.

@jdarwood007
Copy link
Member

This is why for beyond 2.1 I'm recommending we drop support for upgrading anything other than the previous major and build converters for those. Its going to become more troublesome in the future to maintain being able to upgrade from YabbSE all the way to current.

@sbulen
Copy link
Contributor Author

sbulen commented Mar 20, 2020

Yep...

@sbulen
Copy link
Contributor Author

sbulen commented Mar 21, 2020

This is why for beyond 2.1 I'm recommending we drop support for upgrading anything other than the previous major and build converters for those. Its going to become more troublesome in the future to maintain being able to upgrade from YabbSE all the way to current.

ALSO VERY IMPORTANT... We need to be careful what gets pushed down to 2.0... If we push the same changes down to 2.0.x, we will break those, too. Some may be unavoidable (security-related). But we need to be mindful of the impacts.

Signed by Shawn Bulen, bulens@pacbell.net
@sbulen
Copy link
Contributor Author

sbulen commented Mar 25, 2020

Testing completed.

One minor issue outstanding that will be logged separately.

@sbulen sbulen marked this pull request as ready for review March 25, 2020 20:56
@sbulen sbulen added this to the RC3 milestone Apr 6, 2020
@sbulen
Copy link
Contributor Author

sbulen commented Apr 6, 2020

Since some of these issues impact current installs, I think we should merge this one asap.

other/upgrade.php Show resolved Hide resolved
Signed by Shawn Bulen, bulens@pacbell.net
@MissAllSunday MissAllSunday merged commit fd0946a into SimpleMachines:release-2.1 Apr 9, 2020
@sbulen sbulen deleted the upgr_cli branch April 9, 2020 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrader - CLI - custom_avatar & $upgradeData Upgrader - CLI errors
4 participants