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

[4.0] Load autoload_psr4.php and some minor fixes #16680

Merged
merged 2 commits into from Jun 15, 2017

Conversation

yvesh
Copy link
Member

@yvesh yvesh commented Jun 14, 2017

Pull Request for Issue #16616

Summary of Changes

  • Move the namespace map creation / loading to render method in CMSApplication (as suggested by Michael), this should fix the installer issues
  • Load the generated namespace map file
  • Minor fixes in the namespace file

Testing Instructions

Make sure you delete the old autoload_psr4.php in libraries before.

Check that libraries/autoload_psr4.php is generated and paths are loaded.

Expected result

Installation works

Actual result

Installation is broken on default SQL credentials

Documentation Changes Required

Probably

@ghost
Copy link

ghost commented Jun 14, 2017

@yvesh PR for #16616?

@zero-24
Copy link
Contributor

zero-24 commented Jun 14, 2017

Yes was my error -> fixed ;) @franz-wohlkoenig

@yvesh
Copy link
Member Author

yvesh commented Jun 14, 2017

@franz-wohlkoenig yep, fixes that also - updated description. Thanks :-)

@zero-24 zero-24 added this to the Joomla 4.0 milestone Jun 14, 2017
@@ -1025,6 +1023,8 @@ public function redirect($url, $status = 303)
*/
protected function render()
{
$this->createExtensionNamespaceMap();
Copy link
Contributor

@wilsonge wilsonge Jun 14, 2017

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

@wilsonge done

@joomdonation
Copy link
Contributor

Installation works fine now, autoload_psr4.php is generated properly. I tried to exclude Component from JLoader loadByExtension method https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/loader.php#L556 and everything still works well. Some notes if we decide to go with this approach:

  1. There are still some components like com_categories, com_cpanel, com_fields... not registered in autoload_psr4.php

  2. Do we want to support autoload for cli applications? Asking this question because we still register model class manually here https://github.com/joomla/joomla-cms/blob/4.0-dev/cli/finder_indexer.php#L238-L239

@wilsonge
Copy link
Contributor

There are still some components like com_categories, com_cpanel, com_fields... not registered in autoload_psr4.php

They are missing from the install sql script and XML files - easy to fix.

Do we want to support autoload for cli applications? Asking this question because we still register model class manually here https://github.com/joomla/joomla-cms/blob/4.0-dev/cli/finder_indexer.php#L238-L239

Yes - but unsure where the best place to do that is. People may have to implement that manually?

@joomdonation
Copy link
Contributor

They are missing from the install sql script and XML files - easy to fix.
I know. Just wanted to point out so that we won't forget

Yes - but unsure where the best place to do that is. People may have to implement that manually?

I am unsure, too as I haven't written any Cli application. I think it is inconsistent when we have autoload support in Site/Administrator but not in Cli.

@mbabker
Copy link
Contributor

mbabker commented Jun 14, 2017

The problem is all of our existing CLI scripts are all separate applications without any common structure and the last proposal to make something equivalent to a JApplicationCms for CLI basically went nowhere because everything that is not serving HTML responses (JSON views, CLI) is still treated as a second class citizen in Joomla.

@Bakual
Copy link
Contributor

Bakual commented Jun 14, 2017

There are still some components like com_categories, com_cpanel, com_fields... not registered in autoload_psr4.php

They already autoload fine without this autoload_psr4.php. If we want to drop the existing autoload code for the "Joomla Namespace" in favor of this autoload_psr4 file, then we have to add the namespace to the database of those extensions and this script will pick it up. Otherwise it's not needed.

@joomdonation
Copy link
Contributor

@Bakual I know that. But I think the purpose of autoload_psr4.php is to replace the existing autoload code (at least for components part). So if this approach is accepted, we need to register the missing components and drop the existing autoload code.

@yvesh
Copy link
Member Author

yvesh commented Jun 14, 2017

@joomdonation yep, but one step after another :-) Than we can continue updating the entries in the #__extensions table

@joomdonation
Copy link
Contributor

@yvesh As I said, just pointed out so that we won't forget only

@wilsonge wilsonge merged commit 08e94a3 into joomla:4.0-dev Jun 15, 2017
@wilsonge
Copy link
Contributor

Merged so that we have a working installer for people

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants