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

drivers/g15: Always depend on freetype2 #190

Closed
wants to merge 1 commit into from

Conversation

haraldg
Copy link
Collaborator

@haraldg haraldg commented Dec 16, 2021

This is a workaround for an upstream bug. For details see the following
issue. (Closes: #188)

Signed-off-by: Harald Geyer harald@ccbib.org

This is a workaround for an upstream bug. For details see the following
issue. (Closes: lcdproc#188)

Signed-off-by: Harald Geyer <harald@ccbib.org>
@haraldg
Copy link
Collaborator Author

haraldg commented Dec 16, 2021

@toams @jwrdegoede please review and test.

@@ -51,16 +51,14 @@ static inline int g15_send(int sock, char *buf, unsigned int len) { return -1; }
#endif

/*
* If we have freetype2, assume libg15render is build with TTF support,
* Workaround for upstream bug: Assume libg15render is build with TTF support,
Copy link
Member

Choose a reason for hiding this comment

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

change build -> built

Copy link
Member

@ethandicks ethandicks left a comment

Choose a reason for hiding this comment

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

Simple spelling correction in comments. Code is fine.

@@ -51,16 +51,14 @@ static inline int g15_send(int sock, char *buf, unsigned int len) { return -1; }
#endif

/*
* If we have freetype2, assume libg15render is build with TTF support,
* Workaround for upstream bug: Assume libg15render is build with TTF support,
* the TTF_SUPPORT define makes the size of the g15 struct bigger, if we do
* not set this define while libg15render is build with TTF support we get
Copy link
Member

Choose a reason for hiding this comment

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

change build->built

AC_MSG_WARN([The g15driver needs libg15render.h])
])
else
AC_MSG_WARN([libg15render ist broken without freetype])
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: 's/ist broken/is broken/'

@jwrdegoede
Copy link
Contributor

Thank you for doing this, except for the spotted typo the code looks good to me and I've just done a test-build on a Fedora 35 system and the g15 driver still gets properly enabled there (when requested on the ./configure cmdline)

@toams
Copy link
Contributor

toams commented Dec 16, 2021

I got a build error: usr/include/libg15render.h:16:10: fatal error: ft2build.h: No such file or directory.

For some weird reason the includes for freetype aren't found even tough they are installed.
After a lot of googling I was able to fix this build error by running:export CPPFLAGS='-I/usr/include/freetype2'before running ./configure and make. I wonder if this is due a mistake in lcdproc's build system, or maybe something is messed up on my computer.

The good news is that after fixing the compile issues I don't get the segfault anymore!

@haraldg
Copy link
Collaborator Author

haraldg commented Dec 16, 2021

Thanks for the info. Is pkg-config installed on your computer? What's the output of grep pkg-config config.log?

@toams
Copy link
Contributor

toams commented Dec 16, 2021

configure:7351: checking for pkg-config
configure:7369: found /usr/bin/pkg-config
configure:7381: result: /usr/bin/pkg-config
configure:7406: checking pkg-config is at least version 0.9.0
Package libftdi1 was not found in the pkg-config search path.
Package libftdi1 was not found in the pkg-config search path.
Package libftdi was not found in the pkg-config search path.
Package libftdi was not found in the pkg-config search path.
Package libhid was not found in the pkg-config search path.
Package libhid was not found in the pkg-config search path.
ac_cv_path_ac_pt_PKG_CONFIG=/usr/bin/pkg-config
PKG_CONFIG='/usr/bin/pkg-config'

@haraldg
Copy link
Collaborator Author

haraldg commented Dec 16, 2021

I guess for some reason FT2_CFLAGS is not defined for your build. Maybe you can check in config.log. However I don't know enough about autoconf, to debug this.

@jwrdegoede
Copy link
Contributor

@toams can you check what the output of pkg-config --cflags freetype2 is? This output should include -I/usr/include/freetype2.

If it does not include that then there is something wrong with your freetype installation.

@toams
Copy link
Contributor

toams commented Dec 17, 2021

pkg-config --cflags freetype2 outputs -I/usr/include/freetype2 -I/usr/include/libpng16
So I think the problem is in lcdproc's built system.

Do you guys have the freetype2.m4 macro somewhere installed?(try /usr/share/aclocal) Because I did some investigation and I think the problem is that lcdproc's build system uses freetype2.m4 to detect freetypes header files. But freetype2.m4 is obsolete since 2003 or so and its functionality had been replaced by pkg-config. And debian removed freetype2.m4 from the libfreetype-dev package in 2018.
Changing the following lines in configure.ac made the build system detect the header files:

--- a/configure.ac
+++ b/configure.ac
@@ -390,14 +390,14 @@ AC_ARG_ENABLE(freetype,
 AC_MSG_RESULT($enable_freetype)
 
 if test "$enable_freetype" = "yes"; then
-       ifdef([AC_CHECK_FT2],
-               [AC_CHECK_FT2([],
+       ifdef([PKG_CHECK_MODULES],
+               [PKG_CHECK_MODULES([FT2], freetype2,
                        [AC_DEFINE(HAVE_FT2, [1], [Define to 1 if you have freetype])],
                        [enable_freetype=no])],
                [AC_MSG_WARN([freetype does not seem to be installed])])
 fi
-AC_SUBST([FT2_CFLAGS])
-AC_SUBST([FT2_LIBS])
+AC_SUBST(FREETYPE2_LIBS)
+AC_SUBST(FREETYPE2_CFLAGS)
 
 
 dnl ######################################################################

Please note that I am far from an expert on autoconf, I just tried some things I found on google while investigating this problem. Probably an autoconf expert should proof read

@haraldg
Copy link
Collaborator Author

haraldg commented Dec 17, 2021

Thanks! I guess that explains everything. I don't think we have an autoconf expert around, so I'd just go with your fix. But we probably need to replace FT2_CFLAGS and FT2_LIBS in all Makefile.am files with the new symbols.

Are you up to reworking this PR? Feel free to submit as your own and would be a great first contribution. (Hopefully I don't embarrass myself by missing previous contributions.)

@toams
Copy link
Contributor

toams commented Dec 18, 2021

I created a PR: #191
I think bug #188 is fixed if both #190 #191 are merged

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.

Segmentation fault when using g15 driver
4 participants