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

[5.1] Joomla Update : extensions check never ends #43226

Closed
wants to merge 89 commits into from

Conversation

conseilgouz
Copy link
Contributor

@conseilgouz conseilgouz commented Apr 7, 2024

Pull Request for Issue #43196.

Summary of Changes

Fix PHP 8.2 deprecated notices during Extensions check while performing Joomla 5.1.0 update

Testing Instructions

Joomla 5.0.3, PHP 8.2 or higher, enable Error reporting to default.
In Joomla Update configuration, set "Potentially incompatible extensions checkbox" to "Show".
Install any non core extension

Update Joomla to 5.1.0-RC shows the Pre-Update Screen.

Actual result BEFORE applying this Pull Request

Extensions check never ends, an error appears in explorer console : uncaught syntaxError..

Expected result AFTER applying this Pull Request

Extensions check works fine

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

conseilgouz and others added 30 commits March 1, 2024 17:39
Missing item name
….php

Co-authored-by: Brian Teeman <brian@teeman.net>
….php

Co-authored-by: Brian Teeman <brian@teeman.net>
Co-authored-by: Brian Teeman <brian@teeman.net>
Co-authored-by: Brian Teeman <brian@teeman.net>
Co-authored-by: Brian Teeman <brian@teeman.net>
Co-authored-by: Brian Teeman <brian@teeman.net>
Co-authored-by: Brian Teeman <brian@teeman.net>
…g.php

Co-authored-by: Brian Teeman <brian@teeman.net>
…g.php

Co-authored-by: Brian Teeman <brian@teeman.net>
…g.php

Co-authored-by: Brian Teeman <brian@teeman.net>
Co-authored-by: Brian Teeman <brian@teeman.net>
…g.php

Co-authored-by: Brian Teeman <brian@teeman.net>
…g.php

Co-authored-by: Brian Teeman <brian@teeman.net>
…g.php

Co-authored-by: Brian Teeman <brian@teeman.net>
remove tabs
add new lines at end of files
@conseilgouz conseilgouz changed the title [5.1] Extensions check stuck #43196 [5.1]Joomla Update : extensions check never ends Apr 7, 2024
@bembelimen
Copy link
Contributor

bembelimen commented Apr 7, 2024

Thanks @conseilgouz for the finding. We have to discuss how we handle this issue best. It need to be fixed asap, but either it will trigger a new 5.0.x or we will move it to 5.1.1. Sadly 5.1.0 is the wrong version for it, but we get it managed :)

Thanks for the finding and the PR.

Edit: just saw that @brianteeman mentioned it before, yes it's #40999 (tbh it's when the trait was implemented in 4.3 and was not replacing the full functionality). The dependency of stdClass was removed by this PR, so we have to think how to solve it.

@laoneo
Copy link
Member

laoneo commented Apr 8, 2024

@conseilgouz can you rename the variable jversionfull to $targetVersion? Like that it is more meaningful. Also remove the set and get calls and make a proper setter function like `setTargetVersion for it with the? Thanks...

@conseilgouz
Copy link
Contributor Author

@laoneo : ok for targetVersion. In progress...
What about other variables that have been added "the hard way" : downloadurl, tag, stability... ?
It's not the purpose of this PR, but could be cleaner

libraries/src/Updater/Update.php Outdated Show resolved Hide resolved
libraries/src/Updater/Update.php Outdated Show resolved Hide resolved
libraries/src/Updater/Update.php Show resolved Hide resolved
@laoneo
Copy link
Member

laoneo commented Apr 8, 2024

@laoneo : ok for targetVersion. In progress... What about other variables that have been added "the hard way" : downloadurl, tag, stability... ? It's not the purpose of this PR, but could be cleaner

You mean to make getter and setter functions for them?

@conseilgouz
Copy link
Contributor Author

@laoneo : ok for targetVersion. In progress... What about other variables that have been added "the hard way" : downloadurl, tag, stability... ? It's not the purpose of this PR, but could be cleaner

You mean to make getter and setter functions for them?

Well, I was thinking about this, but it's tricky to find the "getters" and the variables are set in _endElement thru a nice foreach (get_object_vars($this->latest) as $key => $val) {
$this->$key = $val;
}
So the setters are also not that easy to create.

@conseilgouz
Copy link
Contributor Author

Everything seems to be ok (thank you @richard67, @laoneo for the time you spent ), but, it applies to Joomla 5.1 when initial issue was about "J5.0.3 update to J5.1.0 does not work".
Did it worth the time we spent ?

@richard67
Copy link
Member

@conseilgouz Well we can’t fix it for 5.0.3 with a CMS update, and we have a workaround for this, switching off error reporting, but we need the fix also for later updates from 5.1.0 to e.g. 5.1.1 in future, so it is ok for 5.1.

@HLeithner HLeithner changed the title [5.1]Joomla Update : extensions check never ends [5.1] Joomla Update : extensions check never ends Apr 24, 2024
@conseilgouz conseilgouz closed this by deleting the head repository Apr 30, 2024
@richard67
Copy link
Member

@conseilgouz You‘ve deleted your fork of the joomla repository, so this PR was closed. Was this a mistake or by purpose?

@conseilgouz
Copy link
Contributor Author

Hi Richard,
I had 2 other PR's which said I could delete my fork and I forgot about this one.
I thought it was already closed.
need a "refork" ?

@richard67
Copy link
Member

need a "refork" ?

@conseilgouz I think so. The PR was not closed by us, and it was approved by @laoneo , so it would have just needed 2 human tests. If you still want to provide a PR, I think you have to fork again and redo the PR.

@richard67
Copy link
Member

New PR is #43410 . @laoneo Could you approve that one, too? It’s the same as this one here which had been closed by mistake.

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.

None yet

7 participants