I hit a problem when using FeedAPI with Activitystream (which also includes the simplepie.inc file). Using both modules causes a PHP error, as the simplepie declarations are attempted twice. Require_once doesn't handle this because there are two different copies of simplepie.inc.

I have got around this my using the simplepie module that is in contrib and adapting FeedAPI simplepie parser, and activitystream to both require this module.

The required change for FeedAPI is to add this to parser_simplepie.info

dependencies[] = simplepie

in parser_simplepie.module change the line that requires the simplepie.inc file to this:

  require_once(drupal_get_path('module', 'simplepie') .'/lib/simplepie.inc');

and in simplepie.install make similar changes to the simplepie.inc require (this appears twice in the install file):

  $path = drupal_get_path('module', 'simplepie') .'/lib/simplepie.inc';

Comments

aron novak’s picture

aron novak
Primary language Hungarian
Location Hungary, Budapest
commented

This suggestion does make sense. I suggest to accept the changes. Any possible drawbacks?

darrenmothersele’s picture

darrenmothersele commented

I made similar changes to the Activitystream module here http://drupal.org/node/303765

Maintainer there raised issue that adding extra module requirement makes an extra barrier to installation. The proposed solution was to check for simplepie Drupal module, and if it exists use that copy, else include own local copy. I'm not totally happy with this as it adds extra unnecessary weight to the module.

Freso’s picture

Freso commented
Version: 6.x-1.3 » 6.x-1.x-dev
Component: Code » Code parser_simplepie

Just a note, instead of calling manually looking for the SimplePie Core module path, the module actually provides a function for including the SimplePie library: simplepie_require()

Also note that a patch has already gone in, which check whether the library is already loaded, so this issue might not be relevant any more (#311039: Use simplepie project instead of including own version).

alex_b’s picture

alex_b commented

We need a patch for this.

alex_b’s picture

alex_b commented
Status: Needs work » Postponed

I just had a look at SimplePie module and saw that the last commit is from September 08. I saw Freso's request for a 1.0 version #454998: 1.0 release. I haven't seen mustafau around on FeedAPI or aggregator issue queue in a while either. As long as the responsiveness of the maintainer doesn't really change, I'd like to postpone this request.

Please feel free to activate this issue once we have reason to believe that SimplePie module is actually actively maintained. For my part - I'd like to see a couple of months sustained activity on the module or of the maintainer to be convinced.

Otherwise, I think this is a great request.

Freso’s picture

Freso commented

I don't really think this is one of those modules that require a high activity level from the maintainers side. Basically, it just requires the SimplePie library and is done. This hardly requires a lot of activity. It just needs to be ported to a new Drupal every once in a while, and that should be it. Library updates have to be done manually anyway. Just something to keep in mind. :)

alex_b’s picture

alex_b commented

Freso:

c'mon, there is no such a thing as a module that doesn't require maintenance :-P

Seriously, I don't want to have the risk of relying on an unmaintained module for such an important part of FeedAPI - no matter how simple the module is :-)

alex_b’s picture

alex_b commented

Freso: please ping me on IRC if you have more questions around this issue...

Freso’s picture

Freso commented
Status: Postponed » Active

I'm fairly confident the new co-maintainer of the SimplePie module will help with keeping the module actively developed... :)

alex_b’s picture

alex_b commented

Ok,

I'd love to loosely couple feedapi and simplepie module: parser_simplepie should first check whether simplepie module is installed, if not, try to include simplepie.inc from its own directory (like it does now). The big gain would be that existing installations don't need to change on a minor version upgrade (less hassle, less issues on queue), while the original issue described here is solved.

Further, there needs to be an update to the README.txt file. I just saw that there is no hook_requirements at all - it would be great if we got a hook_requirements implementation with this patch, too :-)

eyecon-1’s picture

eyecon-1 commented

There is a simpler solution with less overhead. Have feedapi look for simplepie in sites/all/libraries. This is the scheme that WYSIWYG is using for TinyMCE. No additional module required, less code, less maintenance.

aron novak’s picture

aron novak
Primary language Hungarian
Location Hungary, Budapest
commented
Status: Active » Fixed 
if (!class_exists('SimplePie')) {
    require_once(drupal_get_path('module', 'parser_simplepie') .'/simplepie.inc');
  }

Solved in this way.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.