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

Fix config.w32 #5

Closed
wants to merge 2 commits into from
Closed

Fix config.w32 #5

wants to merge 2 commits into from

Conversation

weltling
Copy link

Session ext has to be enabled for the msgpack ext to work properly.

@laruence
Copy link
Member

Hey, Anatoliy, could you explain a little more what the bug is? :)

thanks

@weltling
Copy link
Author

Hi Hui,

msgpack has references to the session extension. When php compiled with "--disable-all --enable-cli --enable-msgpack=shared" a link error is to see. Not able yet to compile with the dev pack on win as there's a dependency.

A better solution might be to add something like --enable-msgpack-session , but this would require to isolate the appropriate places in the code using HAVE_PHP_SESSION macros. I'd see that rather not necessary as session is one of the mandatory core extensions. Of course there are chances someone excludes the session ext in a custom build, but normally session is statically compiled (at least on win) into PHP.

Cheers

Anatoliy

@laruence
Copy link
Member

thanks, but thinking of the situation that seesion is built as a share extension.

I'd say, maybe we should disable session serializer handler, if the session is not enabled. :)

thanks

@weltling
Copy link
Author

Yep, that's what I've meant above - if session ext isn't compiled statically or isn't loaded, currently there's clearly a fail in msgpack. However the patch I've sent will suffice in 99.8% of the cases under windows as session is statically compiled in released php binaries.

The session handler should obviously be disabled at the run time if the session ext isn't available. But that's just another question - compile vs. run time. IMHO it should be always compiled with the session support as on windows most users just use the precompiled dlls.

Cheers )

@laruence
Copy link
Member

fixed in 21b22bf

thanks

@laruence laruence closed this Sep 25, 2012
@joeyhub joeyhub mentioned this pull request May 18, 2015
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.

None yet

2 participants