Smarty debundling

mdev

Joined: 2005-12-08
Posts: 17
Posted: Thu, 2005-12-08 21:42

Is there a technical reason (as opposed to convenience) that smarty is bundled with Gallery2? I'd like to be able to centralize smarty dirs and only have one version of smarty to worry about with system upgrades.

I do realize that "clear cache" user-interface might stop working, but that's a trade-off very much worth it. I can manage with rm -rf just fine :p

 
ckdake
ckdake's picture

Joined: 2004-02-18
Posts: 2258
Posted: Thu, 2005-12-08 22:58

We provide smarty with G2 because its required for G2 to work and the version we distribute is the one that we test with and ensure that everything is working properly. We try to have as few requirements as possible for a system to be capable of running G2 and to require a certain version of smarty for a certain version of G2 would be more of a hassle.

So it's not specifically a "technical" reason, but easy for users + testable by us is enough of a reason.

 
mdev

Joined: 2005-12-08
Posts: 17
Posted: Fri, 2005-12-09 13:19

In other words, I can safely remove the smarty dir as it's unmodified and no hard references to an existing smarty within the gallery installation are there?

 
valiant

Joined: 2003-01-04
Posts: 32509
Posted: Fri, 2005-12-09 15:51

- it is unmodified, at least as far as i can remember
- of course there are references to smarty in g2.

if you absolutly need to remove it from g2...

- change the include path in modules/core/classes/GallerySmarty.class at the top.
- leave the lib/smarty_plugins/ folder where it is
- make sure your smarty version is the same or newer as our smarty version
- make sure your smarty version has a superset of the plugins that are in our lib/smarty/plugins/ folder

 
mdev

Joined: 2005-12-08
Posts: 17
Posted: Fri, 2005-12-09 20:13
valiant wrote:
- of course there are references to smarty in g2.

Why "of course"?
You could just use "GalleryCoreApi::requireOnce" instead of using the relative one, especially since the relative one isn't very smart (see attached patch).

If you wanna do it gracefully, you make smarty_dir a site admin value, or use include_once with an if construct (test for Smarty.class.php, then for lib/smarty/Smarty.class.php and bail out pointing to Mr. CannotReadTheAdmin).

This doesn't add to support problems, as a standard user gets the bundled smarty and anyone brave enough to centralize, gets no pay for a week if he screws up.

Quote:
- leave the lib/smarty_plugins/ folder where it is
- make sure your smarty version has a superset of the plugins that are in our lib/smarty/plugins/ folder

Ok, that doesn't make sense. Why keep if I need a superset in my own plugins folder? In fact, that would clash, as you add the smarty_plugins dir to the smarty search path for plugins in GalleryTemplate.class::_getSmarty().

 
valiant

Joined: 2003-01-04
Posts: 32509
Posted: Fri, 2005-12-09 21:17

@http://gallery.menalto.com/files/GalleryCoreApi.class.diff.txt:
nice finding.
this was on Bharat's (the main dev of g2) radar already and since we're currently in a cvs BRANCH to change the API anyway, we'll change this too.
thanks for reminding us!

what we will do:
- drop GalleryCoreApi::requireOnce (it isn't used in the code anywhere else, it was planned to be dropped)
- rename GalleryCoreApi::relativeRequireOnce to ::requireOnce (analogy: the gallery2 folder is just the virtual include path)
- probably we'll add a static var to keep the gallery2 path as you suggest

for future patches (and i hope you'll submit more :)), please diff against the current cvs version.
(The GalleryCoreApi::relativeRequireOnce function has changed a little since your 2.0/2.0.2. But no need to do submit a new diff, the idea is clear.)

@smarty:

Quote:
You could just use "GalleryCoreApi::requireOnce" instead of using the relative one, especially since the relative one isn't very smart (see attached patch).

ok, we know now that requireOnce will be dropped. we never intended direct use of this function. All our includes are absolute filenames in the end.
so we won't change that just for 3rd party libs that might be available somewhere else in the include path.

but i agree that we could add a check for the presence of smarty / adodb in the existing include_path either during installation or / and in a site admin page and we could store the path in the db and GalleryDataCache cached.
i'd be happy to review a corresponding patch ;)

Quote:
Why keep if I need a superset in my own plugins folder? In fact, that would clash, as you add the smarty_plugins dir to the smarty search path for plugins in GalleryTemplate.class::_getSmarty().

lib/smarty/plugins/ are the official smarty plugins
lib/smarty_plugins/ are own smarty plugins that extend smarty. so we separated the vanilla smarty dir from our own smarty extension.
all i say is: don't delete lib/smarty_plugins/ since it's our own stuff. your own smarty installation should be a 100% replacement for lib/smarty/ and you should make sure that your smarty's plugins/ dir has at least the same plugins as the ones that are in our lib/smarty/plugins/ dir.

 
mdev

Joined: 2005-12-08
Posts: 17
Posted: Fri, 2005-12-09 22:34
valiant wrote:
@http://gallery.menalto.com/files/GalleryCoreApi.class.diff.txt:

what we will do:
- drop GalleryCoreApi::requireOnce (it isn't used in the code anywhere else, it was planned to be dropped)
- rename GalleryCoreApi::relativeRequireOnce to ::requireOnce (analogy: the gallery2 folder is just the virtual include path)

Hmm, why rename? Seems more reasonable to deprecate ::requireOnce and modify relativeRequireOnce, so modules don't break.

Quote:
for future patches (and i hope you'll submit more :)), please diff against the current cvs version.

Noted. I might just start with documenting the Module API properly, as it now is a listing of tools and what the tools do, but nobody explains how one can accomplish a certain task. It's like a car manual listing each component but not telling you how to change a tire. Have to read up on what phpDocumentor does and does not do - I prefer Doxygen :p.

Also, is the archiveupload module up for review, cause ... well, cause of a lot, but mostly, there's no hooks to add archive formats without writing a new module and/or rewriting archiveupload. I'd like to split it, remove any hard refs to 'zip*', replace unzipPath with 'extractPath', get rid of the various unneeded foreach loops with hard-coded one item arrays, and add an autoreader for a formats/ subdir where zip|rar|tar.class can reside and register itself in archiveupload.

But, I still have to figure out if modules in modules can register their own properties and be configured/enabled/disabled accordingly.

Quote:
@smarty:
...

but i agree that we could add a check for the presence of smarty / adodb in the existing include_path either during installation or / and in a site admin page and we could store the path in the db and GalleryDataCache cached.
i'd be happy to review a corresponding patch ;)

Not sure if caching something that only needs to be established once per request is that big of a gain, when the trade off is, that it breaks rather ungracefully if smarty was moved and include_path modified accordingly. Especially when you wanna test compatibility before upgrading - it generates a false error.

Quote:
lib/smarty/plugins/ are the official smarty plugins
lib/smarty_plugins/ are own smarty plugins that extend smarty.

Right, didn't see the _ and / difference.

 
valiant

Joined: 2003-01-04
Posts: 32509
Posted: Fri, 2005-12-09 22:54
Quote:
Hmm, why rename? Seems more reasonable to deprecate ::requireOnce and modify relativeRequireOnce, so modules don't break.

yeah, on the other hand, if we have just a single requireOnce/relativeRequiriceOnce function, then better choose a simple name, requireOnce.

Quote:
Noted. I might just start with documenting the Module API properly, as it now is a listing of tools and what the tools do, but nobody explains how one can accomplish a certain task. It's like a car manual listing each component but not telling you how to change a tire. Have to read up on what phpDocumentor does and does not do - I prefer Doxygen :p.

yep, we don't have a manual. neither a user manual nor a development manual. the phpdoc is just a API doc, documenting each method / class.
a real development manual would be huge for the project. (meanwhile, h0bbel++ are planning and beginning with the user manual).
i've written down a few notes at:
http://gallery.menalto.com/node/36885#comment-134279

do you really thing phpdoc / doxygen can be used for a development manual? wouldn't our codex.gallery2.org be better suited for a manual?

@archiveupload:
yep, refactoring it to allow for multiple archive formats sounds good.

Quote:
But, I still have to figure out if modules in modules can register their own properties and be configured/enabled/disabled accordingly.

not sure what you mean exactly, but yes, modules can autoconfigure, you can have a configuration page in site admin, you can add any options, db tables, register factory implementations, ...

@smarty include:
yeah, didn't think of the details yet. maybe just store a flag whether to use an open include path or our absolute path.

 
mdev

Joined: 2005-12-08
Posts: 17
Posted: Fri, 2005-12-09 23:33
valiant wrote:
Quote:
Hmm, why rename? Seems more reasonable to deprecate ::requireOnce and modify relativeRequireOnce, so modules don't break.

yeah, on the other hand, if we have just a single requireOnce/relativeRequiriceOnce function, then better choose a simple name, requireOnce.

Bleh you linux api-breaking people :p
At least keep relativeRequireOnce till 3.0, since you advocated it's use and 3rd party modules break instantly.

Quote:
do you really thing phpdoc / doxygen can be used for a development manual? wouldn't our codex.gallery2.org be better suited for a manual?

I don't like wiki's for dev manuals. The references in apidocs like phpdoc or Doxygen are very useful. It can be used for how-to's though and walk-through guides. Not sure about phpdoc, but Doxygen has groups - you can make group based documentation and add files to a certain group.
What's missing in these docs, is the use of an intro for the module in question.
For example:
/**
* Module meta-info container
*
* This is a container for information about a given module.
*
* @package GalleryCore
* @subpackage Classes
*/

Could be changed to:
/**
* @brief Module meta-info container
*
* Contains all the interfaces needed to add a module to gallery.
* Since it subclasses GalleryPlugin you might find some extra
* functionality there, but most should be available here.
* @h1 General Operation
* All modules should at least provide foo bar baz methods.
* @h2 Adding a module
* A module package needs to include the following files:
* @li MANIFEST
* @li modulename.class containing the class that extends GalleryModule
* @li ...
*
*
* @package GalleryCore
* @subpackage Classes
*/

Well, you get the idea. The principle of source driven documentation is that docs are in sync with source, and having a good header/intro like that, is a good way for developer to see how his changes affect the whole, plus it's easier for new developers to step in.

valliant wrote:
@archiveupload:
yep, refactoring it to allow for multiple archive formats sounds good.

Quote:
But, I still have to figure out if modules in modules can register their own properties and be configured/enabled/disabled accordingly.

not sure what you mean exactly, but yes, modules can autoconfigure, you can have a configuration page in site admin, you can add any options, db tables, register factory implementations, ...

Yes, a module has that, but can you like "enable/disable archive->rar and archive->zip" independantly via the site admin. Like I said, I'll look into it.

 
valiant

Joined: 2003-01-04
Posts: 32509
Posted: Fri, 2005-12-09 23:57

@requireOnce:
yep, it will be changed in the BRANCH_API_DEV (api v.3.0 / Gallery 2.1).
there are so many other changes, this will be a small change compared to the others. yeah, sometimes you must break things to get them right. and g2 is still far too slow, includes far too many files etc. that's why we need to break things.

@source code driven documentation:
yeah, i see that it has its uses.
but i don't see how you can create a task or even process oriented documentation with source driven documentation. you'd have to put most of the documentation or at least a huge index into the GalleryModule class.

@enable/disable archive->rar / archive->zip:
there's no such thing yet. if your module introduces this abstraction of multiple archive toolkits, then you'd have to manage them.

 
mdev

Joined: 2005-12-08
Posts: 17
Posted: Sat, 2005-12-10 12:01

Hmm, yeah, I see the prob. A module can only enable or disable, not turn on/off sub-features.
However, what I'm missing here, is if autoconfigure is successful, I can't change any of it's settings. There's no 'view configuration' in the site admin, allthough that shouldn't be too hard to work in.

@breaking stuff
Here's a bold one: make g3 php5 only. There's so many things in G2 to just enable proper OO behavior, which are already in php5, that's the bigger bottleneck.

 
valiant

Joined: 2003-01-04
Posts: 32509
Posted: Sat, 2005-12-10 19:40

@php5 only / g3:
there are no specific plans for G3 yet (maybe in bharat's head). g2.1 (api 3.0) will be released in february 2006.
there are still far too many php4 based webhosts and it may still take two years until the majority of the webhosts use php5. so php5-only isn't an option for G2 for quite some time.
yes, php4's OO support / implementation is flawed and we would profit from PHP5's features, but can live with it.
the changes in api 3.0 will be very drastic (no specifc map classes anymore, no interface classes, std classes instead of specific entity classes (no need to load the whole class tree anymore), change from inheritance to composition for our entity classes, ...) and there other a lot of other changes too.