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

Linux build issues #7

Open
cmb69 opened this issue Oct 22, 2024 · 5 comments
Open

Linux build issues #7

cmb69 opened this issue Oct 22, 2024 · 5 comments

Comments

@cmb69
Copy link
Contributor

cmb69 commented Oct 22, 2024

I tried to build the extension on Linux (actually WSL 2 with Debian bookworm), but that failed because I don't have md4c-dev installed:

checking whether to enable MD4C support... yes, shared
checking for md4c headers... not found
configure: error: Please install md4c

Since md4c-dev is not readily available on Debian bookworm, I tried building with the bundled md4c, and changed config.md4 to:

dnl config.m4 for php-md4c extension

PHP_ARG_WITH(md4c, [whether to enable MD4C support],
[  --with-md4c[[=DIR]]       Enable MD4C support.
                          DIR is the path to MD4C install prefix])

if test "$PHP_MD4C" != "no"; then
	PHP_SUBST(MD4C_SHARED_LIBADD)
	AC_DEFINE(HAVE_MD4C, 1, [ ])
	PHP_NEW_EXTENSION(md4c, md4c.c, $ext_shared)
fi

Afterwards the build succeeded:

$ phpize
$ ./configure --with-md4c
$ make
$ make test

Now I thought that the build probably cannot succeed with a system md4c library, since it is already contained in md4c.c. So either you can completely drop support for building against a system md4c, or would need to put the md4c amalgamation in a separate file, and only use this if the system library is not available. I assume that distro maintainers prefer the latter (to be able to build against system libraries). Maybe @remicollet can clarify?

@eklausme
Copy link
Owner

eklausme commented Oct 22, 2024

Sorry, I should have taken out any reference to header or libraries for building. That was the whole point of amalgamation, so there is no dependency. In particular Windows won't have MD4C installed.

Also, line 7:

if test "$PHP_YAML" != "no"; then

should use $PHP_MD4C instead.

@cmb69
Copy link
Contributor Author

cmb69 commented Oct 22, 2024

In particular Windows won't have MD4C installed.

Right, but that dependency could be made available on https://downloads.php.net/~windows/pecl/deps/, and could be put in the Windows builds of md4c like for other extensions (e.g. imagick), or built statically into php_md4c.dll.

@eklausme
Copy link
Owner

I committed your proposed config.m4.

@cmb69
Copy link
Contributor Author

cmb69 commented Oct 22, 2024

Okay, might be best to have the library bundled for now, because it has not reached version 1.0, so some things might change.

@remicollet
Copy link
Contributor

I think both (bundled and system) should be allowed

For system usage, see PR #14

For bundled case, I think it could be cleaner to use a git submodule, fetching last tag
of course, sources/doc need to be included in package.xml, and build command (+ proper set of includedir and builddir)

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

No branches or pull requests

3 participants