Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Proposal: Add ability to auto-discover custom libraries #1738

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Contributor

dongilbert commented Dec 4, 2012

See discussion here: https://groups.google.com/forum/?fromgroups=#!topic/joomla-dev-platform/cl_73WKCkgk

See this gist: https://gist.github.com/4208509

I propose adding the ability to auto-discover custom libraries found in the /libraries directory. Libraries can be auto-discovered by including a setup.php file in the root of the library folder. If you wanted to add the SwiftMailer library, you could place a swift folder containing the library in your /libraries folder. The /libraries/swift folder would contain a setup.php file in it which, when found, would be included and executed.

We could also set a standard as to the basic structure of the setup.php file, but that's beyond the scope of this PR.

Contributor

LouisLandry commented Dec 5, 2012

I must admit I prefer using JLoader to register the class paths you are going to use explicitly. I don't really like the idea of blind includes based on a path. Especially in a startup part of application execution. I'm immediately drawn to quite a few attack vectors just by putting a file in a specified place. Your app doesn't have to even ask for it, it gets included anyway.

All that being said, I am open to it if this is something that a lot of people want. I just don't know that I would use it in my own stuff. I'd be very interested to hear what others think on the matter.

Contributor

dongilbert commented Dec 5, 2012

Ya - I see your point about blindly including libraries based on the presence of a particular file - an advanced hacker with knowledge of the system would probably find that very useful. :(

Thinking out loud here, but what if this were modified to autoload a json config file that would specify which libraries to automatically set up? Instead of blindly including every setup.php it finds, you would explicitly set which custom libraries to load.

My end goal here is to be able to easily add libraries / bundles / packages to the libraries folder, and have them "just work". Basically drop in files, and then start using the classes; all without having to worry about something being overwritten with an update.

Contributor

dongilbert commented Dec 5, 2012

OR (here's a thought) we add a new protected var in the JConfig for the site which specifies which libraries to autoload.

class JConfig
{
    ...
    protected $autoloadLibraries = array(
        'swift',
        'stripe',
        'moustache',
        'joomla' // just throwing the idea in here.
    ); 
}

The configuration for a site (in the CMS at least) does not get overwritten with an update, so it would be a good spot to place it in that situation.

Contributor

elinw commented Dec 5, 2012

I guess I'm not sure that I think it's configuration even though configuration files are handy places to put things in a development environment. Configuration to me is something that ... you use to configure the application i.e. you might change it or people with different instances might have different values. But for a specific application I don't don't think it would normally be adding new libraries unless it's really a specific kind of application that's for adding libraries.

Contributor

dongilbert commented Dec 5, 2012

Popular development frameworks like CodeIgniter/Laravel/FuelPHP and others do something similar to this to auto-load their sparks/bundles/packages. They have a $autoload param (or similar) in the app configuration that instructs the app on what to load. It's a convenient way to include custom libraries without too much hassle. It's part of the ease of use "magic" of the framework, if you will.

http://ellislab.com/codeigniter/user-guide/general/autoloader.html
http://laravel.com/docs/bundles
http://fuelphp.com/docs/general/packages.html

Contributor

elinw commented Dec 5, 2012

I like the way that the CMS requires that libraries be installed into the extensions table (which I'd love to rename) in order to be used. In that sense when there is a replacement for the old installer classes we could think about still having discover install for libraries. The advantage is that people know what they are getting into have to make a conscious decision about as opposed to having whatever files happen to have landed there. I can see a number of situations in which there are side by side applications all using the same libraries folder and some needing one library (say the cms library) and some not. I'm not against doing something I just don't love the idea of doing in in the configuration file, which I think is real world going to be normally accessed by some kind of jform that loops through the variables. Do we really want to resave that every time? Is it going to change? It's not like users will pick and choose.

Contributor

LouisLandry commented Dec 6, 2012

@dongilbert What is so different about:

// Bootstrap the Joomla Platform.
require_once __DIR__ . '/libraries/import.php';

// Include other dependencies.
include JPATH_PLATFORM . '/swift/setup.php';
include JPATH_PLATFORM . '/stripe/setup.php';
include JPATH_PLATFORM . '/moustache/setup.php';

or even

// Bootstrap the Joomla Platform.
require_once __DIR__ . '/libraries/import.php';

// Include other dependencies.
JLoader::registerNamespace('Swift', JPATH_PLATFORM . '/swift');
JLoader::registerNamespace('Stripe', JPATH_PLATFORM . '/stripe');
JLoader::registerNamespace('Moustache', JPATH_PLATFORM . '/moustache');

than

class JConfig
{
    ...
    protected $autoloadLibraries = array(
        'swift',
        'stripe',
        'moustache',
        'joomla' // just throwing the idea in here.
    ); 
}

I mean, in either of those cases you have to add 3 lines of code to a file. I dunno if using a configuration file makes a ton of sense. We are not really talking about configuring the application in any way, we are talking about importing library dependencies. What happens to the app if I go in and remove the line to autoload stripe from the configuration file? Does it then use some other payment processor library or have I just fatal error crashed the application? In my way of thinking it is likely the latter, but I may be mistaken.

To me, configuration needs to be things to flip switches and pull levers such that the application changes it's behavior, not loading dependencies like that. I feel like if my application depends upon those packages then it should be OK for my bootstrap file (index.php or whatever) to require or register them with JLoader. We really aren't talking about any extra work. Unless I'm missing something...

Contributor

dongilbert commented Dec 6, 2012

@elinw I had completely overlooked the fact that the CMS already supported something like this with the ability to install libraries. I'm probably going to go that route instead.

@LouisLandry You're correct in your assessment there - the app would blow up - so config surely isn't the best place for it. Also, I was using the write name when referring to "autoload" of the other frameworks; they generally call it something like "alwaysload" or "eagerload". Which better describes it's role in the app config - being that these are things that are required for the app to run.

I'm going to close this for now. I still think something should be done - but this is not it. I can see potential issues with this specific implementation, so the PR is no good.

Maybe the discussion about Packager, Composer, Phar, and dependency mangement could lead to a solution - https://groups.google.com/d/msg/joomla-dev-platform/0RtCZafdZy8/q0Y3-aTcj1wJ

@dongilbert dongilbert closed this Dec 6, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment