Skip to content

Conversation

anthonyfok
Copy link
Contributor

@anthonyfok anthonyfok commented Jun 16, 2022

GNU Mach microkernel header files (gnumach-dev 1.8+git20201129) added to
struct i386_xfp_save in /usr/include/i386-gnu/include/mach/i386/fp_reg.h
a new extended field which happens to collide with the macro definition
#define extended double in FontForge, leading to FTBFS on GNU Hurd:

 FAILED: fontforge/CMakeFiles/fontforge.dir/asmfpst.c.o 
 /usr/bin/cc -Dfontforge_EXPORTS -I../../fontforge -I../../inc -Iinc -isystem /usr/include/glib-2.0 -isystem /usr/lib/i386-gnu/glib-2.0/include -isystem /usr/include/freetype2 -isystem /usr/include/libxml2 -isystem /usr/include/readline -isystem /usr/include/python3.9 -g -O2 -ffile-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -fPIC -Werror=implicit-function-declaration -Werror=int-conversion -fdiagnostics-color=always -std=gnu99 -MD -MT fontforge/CMakeFiles/fontforge.dir/asmfpst.c.o -MF fontforge/CMakeFiles/fontforge.dir/asmfpst.c.o.d -o fontforge/CMakeFiles/fontforge.dir/asmfpst.c.o -c ../../fontforge/asmfpst.c
 In file included from ../../fontforge/asmfpst.h:4,
                  from ../../fontforge/asmfpst.c:30:
 ../../fontforge/splinefont.h:63:18: error: two or more data types in declaration specifiers
    63 | #define extended double
       |                  ^~~~~~
 In file included from /usr/include/i386-gnu/bits/sigcontext.h:30,
                  from /usr/include/signal.h:291,
                  from /usr/include/glib-2.0/glib/gbacktrace.h:36,
                  from /usr/include/glib-2.0/glib.h:34,
                  from /usr/include/glib-2.0/gobject/gbinding.h:28,
                  from /usr/include/glib-2.0/glib-object.h:22,
                  from /usr/include/glib-2.0/gio/gioenums.h:28,
                  from /usr/include/glib-2.0/gio/giotypes.h:28,
                  from /usr/include/glib-2.0/gio/gio.h:26,
                  from ../../inc/ffglib.h:29,
                  from ../../fontforge/baseviews.h:31,
                  from ../../fontforge/fontforgevw.h:31,
                  from ../../fontforge/asmfpst.c:33:
 /usr/include/i386-gnu/mach/i386/fp_reg.h:80:24: error: expected identifier or ‘(’ before ‘[’ token
    80 |  unsigned char extended[0]; /* Extended region */
       |                        ^

This issue may be circumvented by temporarily undefining the extended macro before loading
<gio/gio.h> and friends, and redefining the extended macro right after, as is done in this PR.

For Debian packaging, in fontforge (1:20201107~dfsg-4) released 2021-01-15, I replaced all relevant instances of extended with extendeddbl to avoid macro/type name collision; see my patch at https://sources.debian.org/src/fontforge/1%3A20201107~dfsg-4/debian/patches/0005-hurd-rename-extended-to-avoid-conflict-with-gnumach-dev.patch/

However, when it came time to upgrade to fontforge (1:20220308~dfsg-1) on 2022-06-16, I realized such a patch would be very hard to maintain in the long run. Fortunately, I came across instances of #undef extended in the existing codebase for avoiding conflict with xlink.h, and borrowed the idea for this pull request. I am happy to report that this allows fontforge to build successfully on hurd-i386; see https://buildd.debian.org/status/logs.php?pkg=fontforge&ver=1%3A20220308%7Edfsg-1&arch=hurd-i386

Type of change

  • Bug fix
  • Non-breaking change

GNU Mach microkernel header files (gnumach-dev 1.8+git20201129) added to
struct i386_xfp_save in /usr/include/i386-gnu/include/mach/i386/fp_reg.h
a new "extended" field which happens to collide with the macro definition
"#define extended double" in FontForge, leading to FTBFS on GNU Hurd.

This issue may be circumvented by temporarily undefining the "extended"
macro before loading `<gio/gio.h>` and friends, and redefining the
"extended" macro right after.
Copy link
Member

@ctrlcctrlv ctrlcctrlv left a comment

Choose a reason for hiding this comment

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

Ugh, what a mess. Way too common of a name, but it's used extensively:

$ find . -iname '*.c' -or -iname '*.h'|xargs grep 'extended'|wc -l
324

As you note, we already have to undef it for another collision:

$ rg 'undef extended'
fontforge/svg.c
1140:#undef extended			/* used in xlink.h */

fontforge/ufo.c
60:#undef extended			/* used in xlink.h */

So I see no reason not to merge this; I doubt anyone is going to want to tackle unfudging more than 20 years of brokenness in FontForge's floating point types.

@ctrlcctrlv ctrlcctrlv merged commit e0480f1 into fontforge:master Jun 17, 2022
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.

2 participants