[#18820] - Allow vendor usage for libraries
- Fixed in Code Base
- 17 Mar 2018
- Medium
- Build: staging
- # 18820
- Diff
- phproberto:lib-vendor
User tests: Successful: Unsuccessful:
Currently libraries are intended to be stored in /libraries/{NAME} which can lead to conflicts between library names and can make the libraries folder a total mess if you have a lot of libraries installed.
Wouldn't be awesome that developers could use it's own vendor library folder? Something like what we already use for libraries/joomla that contains a set of "sub-libraries".
The main idea is that devs could have a folder with an structure like:
librariesphprobertomy_libraryanother_library
Seeing that loader already supports loading such libraries I thought that it shouldn't be complicated so I decided to play with it a couple of hours. Sample loading calls:
// How we are already doing it
JLoader::import('joomla.filesystem.file');
// How a vendor library class could be loaded
JLoader::import('phproberto.my_library.some_class');Summary of Changes
This modifies the library installer to allow that manifests can use:
<libraryname>phproberto/sample</libraryname>instead of:
<libraryname>sample</libraryname>I was surprised when I saw that the installer already supports my idea partially. The library installer is already exploding the libraryname to copy files into a subfolder:
So this only modifies the installer to update manifest destination, discover install, load language string and extension appearance in extension manager.
This pull request also fixes a side bug I found when testing that everything works correctly: when a library stores language files inside its library folder (example: libraries/phproberto/sample/language) those language files weren't automatically loaded in extension manager. They are loaded for other extensions like components, modules, plugins and templates.
Testing Instructions
I'm providing an sample vendor library for testing purposes:
Test library installation/usage
- Install Sample Vendor Library
- Check that description of the library is translated in the postinstallation message.
- Check that the library is found in
libraries/phproberto/sample - Go to extension manager and check that library is listed correctly.
- In your frontend template index.php file add this lines to test library usage:
// Test library loading
JLoader::import('phproberto.sample.library');
// Test library class usage
$class = new \Phproberto\Joomla\Sample\TestClass;
// This echoes a library language string to test it
echo $class->hello();You should see a message like:
Hello!! This is a sample language string loaded from a vendor library
Test library uninstall
- Go to extension manager and search for
Phproberto. You should see the library listed. - Uninstall the library.
- Ensure that uninstallation works.
Test library discover install
- Go to extension manager and install the sample library from the zip provided.
- In your database editor search for the library in the
#_extensionstable. - Delete the library row.
- Go to extension manager > Discover and click discover.
- Ensure that the extension is shown with its name translated
- Select the extension and click
Install - Ensure that the installation finished correctly.
Additional suggested tests
- Install a third party library to ensure install/uninstall still works
Documentation Changes Required
Update library manifest information to add information to use the vendor folder.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_installer Libraries |
| Labels |
Added:
?
|
||
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18820.
RTC, Thank you @phproberto
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18820.
I've just noticed something which is semi related to this PR @phproberto if you uninstall the library the parent folder is left behind even if it contains nothing. I don't really see it as an issue since it contains nothing, but worth noting nontheless.
@phproberto what will happen if sample is already installed and you install random\sample?
I guess it would be nicer to always require the phproberto/sample structure, and that can only happen in J4
Thanks for testing!!!
@tonypartridge I thought about that when testing it but forgot to invest some time on it after having everything working. Ideally after uninstalling the library the system should be exactly as before installing it. I will send a patch.
@dgt41 that cannot be an issue because you will never have such conflict. lib_sample and lib_phproberto_sample are identified as a totally different extensions. One would have element sample and the other one phproberto/sample. They would be also loaded with a different string because the folder structure.
About forcing this behaviour for v4 I wouldn't recommend it because that will force all the devs to rewrite all their libraries for an aesthetic change. AFAIK v4 is a soft transition so I'd recommend that the old library system works in deprecated mode warning users that v5 will remove support for libraries without vendor.
Great Roberto and completely agree re J4.
On 27 Nov 2017, 21:34 +0000, Roberto Segura notifications@github.com, wrote:
Thanks for testing!!!
@tonypartridge I thought about that when testing it but forgot to invest some time on it after having everything working. Ideally after uninstalling the library the system should be exactly as before installing it. I will send a patch.
@dgt41 that cannot be an issue because you will never have such conflict. lib_sample and lib_phproberto_sample are identified as a totally different extensions. One would have element sample and the other one phproberto/sample. They would be also loaded with a different string because the folder structure.
About forcing this behaviour for v4 I wouldn't recommend it because that will force all the devs to rewrite all their libraries for an aesthetic change. AFAIK v4 is a soft transition so I'd recommend that the old library system works in deprecated mode warning users that v5 will remove support for libraries without vendor.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
I've updated the PR. Now it tries to delete vendor folders on uninstall (manifest & library folders) so we don't have empty vendor folders.
What happens in this scenario
Component 1 installs library x
Component 2 installs library x
Uninstall component 1 now also uninstalls library x leaving component 2 broken
@brianteeman I don't think that's an issue related to this feature. What happens now (without vendors) in the same scenario? Same issue.
Devs already have to take care of those things.
iirc currently the component uninstall doesnt remove the library (or they chose not to)
if i read your comments correctly the library is automatically uninstalled
(maybe i need more coffee but I wanted to raise this to ensure that its not a problem)
I have tested this item
Tested working as it was, but now also deletes the parent directly and only if it's empty. Tested with the provided sample package and duplicating the sample package to test under a group with more than 1 library.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18820.
@brianteeman if that developer is doing that then they should have a check against their component that none of it's other components are installed.
That same issue applies right now even without this patch.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18820.
This isn't really changing current behaviour @brianteeman. Probably what confused you is the folder removal. That's removing the vendor folder when there aren't libraries on it anymore. So if I uninstall phproberto/sample but I don't uninstall phproberto/another_lib it won't be removed. But if I have only a phproberto/sample and I uninstall it you won't have an empty /libraries/phproberto folder which was what @tonypartridge reported.
I'm not changing anything outside that. Library uninstaller already uninstalls libraries (which looks like the right thing :D).
ok - just wanted to be certain and avoid issues down the line
Thanks @brianteeman! Always best to be check for sure :D
On 28 Nov 2017, 09:52 +0000, Brian Teeman notifications@github.com, wrote:
ok - just wanted to be certain and avoid issues down the line
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18820.
| Status | Pending | ⇒ | Ready to Commit |
RTC, down to the release leader for the final decision
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18820.
Thank you very much for testing @dgt41, @tonypartridge & @brianteeman !
mbabker
- | Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-03-17 16:01:17 |
| Closed_By | ⇒ | mbabker | |
| Labels |
Added:
?
|
||
I have tested this item✅ successfully on 9e4107c
Great bit of house keeping :-).
Tested using test instructions provided and also with our own library which we already partially use this method in terms of folder nesting/naming.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18820.