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

Zend: support relative paths for extensions #159

Closed
wants to merge 1 commit into from
Closed

Zend: support relative paths for extensions #159

wants to merge 1 commit into from

Conversation

pcarrier
Copy link

This would simplify greatly puppetization of Zend modules such as xdebug
in heterogeneous environments.

For example:
zend_extension="/usr/lib/php5/20090626/xdebug.so"
can become:
zend_extension=xdebug.so

This would simplify greatly puppetization of Zend modules such as xdebug
in heterogeneous environments.

For example:
zend_extension="/usr/lib/php5/20090626/xdebug.so"
can become:
zend_extension=xdebug.so
@travisbot
Copy link

This pull request fails (merged aa52639 into 253760b).

@pcarrier
Copy link
Author

Looks like a false positive. I pulled again just in case and git 1.7.11.4 rebased like a charm.

@pcarrier
Copy link
Author

BTW, I didn't look into your branching model (not a PHP fan, sorry), but a backport of this feature in 5.3 (which I developed this for) and 5.4 would be most welcome.

Any chance that might happen? Is there a procedure I can follow for that?

@@ -20,6 +20,7 @@
/* $Id$ */

#include "zend_extensions.h"
#include "php.h"

Copy link
Contributor

Choose a reason for hiding this comment

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

It's not a good idea make Zend engine rely on php

Copy link
Author

Choose a reason for hiding this comment

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

php.h is used for DEFAULT_SLASH, IS_SLASH and INI_STR("extension_dir").

Do we have equivalents in the Zend world?

Copy link
Member

Choose a reason for hiding this comment

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

that is one reason of why zend extension does not support relative path.

Copy link
Author

Choose a reason for hiding this comment

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

OK. Could we consider an optional approach based on dlsym?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, including php.h in Zend Engine files is definitely a no go. Same for using any symbols not originating in Zend Engine.

@reeze
Copy link
Contributor

reeze commented Aug 13, 2012

Hi, @travisbot is a bot indeed , and PHP's test on travis-ci.org didn't works that well. you could ignore that for now:)

@php-pulls
Copy link

Comment on behalf of laruence at php.net:

won't fix, it's trivial and fixing this will need over expensive costs

@php-pulls php-pulls closed this Aug 17, 2012
@patcon
Copy link

patcon commented Sep 15, 2012

EDIT: Found reasonable fix after posting. A little hacky, but it works. Still think this is a reasonable request, but not a blocker :)


Not so trivial for config management tools as they are put in developer's hands. Hitting the same issue with chef.

oriuminc/vagrant-ariadne#9

Throws a big red error that is confusing to new users on the first boot of any environment that is provisioning. System's integration tools are getting simpler and simpler to pass to developers, so I can't imagine my project will be the only one it affects. Anyone using xdebug with vagrant (and any config management tool that rightfully passes warnings forward) should see it flash up on the first boot

@Tyrael
Copy link
Contributor

Tyrael commented Sep 15, 2012

https://bugs.php.net/bug.php?id=54564
please close the bug if we don't want this in general.
personally I'm all up for it.

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.

8 participants