Navigation Menu

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

don't build phpdbg by default #3134

Closed
wants to merge 1 commit into from
Closed

Conversation

tvlooy
Copy link
Contributor

@tvlooy tvlooy commented Feb 20, 2018

When compiling readline as shared extension for the cli SAPI, this failed because phpdbg is compiled by default and needs readline. Compiling with or without readline works, only compiling as shared extension makes phpdbg fail.

/usr/src/php/php-src/sapi/phpdbg/phpdbg_cmd.c:765: undefined reference to `readline'
/usr/src/php/php-src/sapi/phpdbg/phpdbg_cmd.c:773: undefined reference to `add_history'
clang: error: linker command failed with exit code 1 (use -v to see invocation)
*** Error 1 in /usr/src/php/php-src (Makefile:293 'sapi/phpdbg/phpdbg')

I don't know why phpdbg is compiled by default. Maybe this was commited by mistake in 378a05f0de7#diff-70ffcc68633eb9f00e5f1aea6023fcdeR6

This PR suggests to turn it off again. Or, maybe we should leave it on by default and need a --disable-phpdbg instead?

@nikic
Copy link
Member

nikic commented Feb 20, 2018

/cc @bwoebi as you enabled it by default.

If we do this, --enable-phpdbg needs to be added to the CI configures.

Regardless of whether or not phpdbg is enabled by default, that readline issue looks like a problem that has to be fixed independently.

@tvlooy
Copy link
Contributor Author

tvlooy commented Feb 20, 2018

@nikic travis/compile.sh already has a --enable-phpdbg

@carusogabriel
Copy link
Contributor

@tvlooy @bwoebi What's the status here?

@tvlooy
Copy link
Contributor Author

tvlooy commented Feb 7, 2019

still relevant. phpdbg is being compiled by default, this PR stops that behavior

The fact that you need readline compiled in when you build with phpdbg is something I personally don't have problems with. I guess that even makes sense

@krakjoe
Copy link
Member

krakjoe commented Mar 27, 2019

The original problem, which is the shared build making phpdbg build fail, is not so easy to fix. SAPI's are configured before extensions, so at configure time for phpdbg, readline is not configured, therefore we cannot detect whether it is shared or static.

EDIT: I found a way ...

@krakjoe
Copy link
Member

krakjoe commented Mar 27, 2019

Fixed in 7af270e

By default readline/phpdbg integration is enabled, and assumes a static build as it does now - this configuration will still fail, as it does now.

To have the build pass with shared readline you must --disable-phpdbg-readline

@krakjoe krakjoe closed this Mar 27, 2019
@tvlooy
Copy link
Contributor Author

tvlooy commented Mar 27, 2019

ok thanks @krakjoe

@petk
Copy link
Member

petk commented Mar 28, 2019

Hello, this seems to fail now:

./buildconf && ./configure --disable-all && make
Undefined symbols for architecture x86_64:
  "_add_history", referenced from:
      _phpdbg_read_input in phpdbg_cmd.o
  "_readline", referenced from:
      _phpdbg_read_input in phpdbg_cmd.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make: *** [Makefile:273: sapi/phpdbg/phpdbg] Error 1

@krakjoe
Copy link
Member

krakjoe commented Mar 28, 2019

Fixed in b7442f1

Now readline support is disabled by default in phpdbg, enabled it when building readline static with --enable-phpdbg-readline ...

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

Successfully merging this pull request may close these issues.

None yet

5 participants