AFAIK this is bug in Debian [0] with patch attached but not applied yet. 

There is similar problem for fribidi, which is now external dependency of
pango. It was fixed upstream in [1].

[0] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=887087
[1] https://github.com/fribidi/fribidi/commit/e1c47ff159bb6853e78dda5a59bb137beceda8d2#diff-c42bd6d70f6353c80cf1f34e4bfa3bc1

Created attachment 368185 [details] [review]
meson: Switch -Werror=undef to -Wundef

On current Debian pango pulls in headers from freetype and fribidi
which trigger this error. As it's not nice to break the build over
this, change things to be less strict for now.

This issue has broken the build for me (using JHBuild on Fedora 27).

Review of attachment 368185 [details] [review]:

No.

The appropriate solution is *not* to make Pango build with random undefined symbols that evaluate to 0.

We're also hitting https://bugs.freedesktop.org/show_bug.cgi?id=82548

A newer fribidi has been added to jhbuild by ebassi, which should no longer trigger this: https://git.gnome.org/browse/jhbuild/commit/?id=f3aa0f0041cf626d2f704587861f44b756de0677

For freetype thing is a Debian only bug it seems, and is tracked here: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=887087

We had a long discussion about this today in #release-team. In the case of Freetype, there are no undefined symbols here, only preprocessor macros. A quick skim of the header indicates the most damage we're likely to see here is that the types of FT_INT64 and FT_UINT64 change to using long long instead of normal long, which doesn't seem like the end of the world.

But that's only one issue. There is also this:

/usr/include/fribidi/fribidi-common.h:35:5: warning: "DONT_HAVE_FRIBIDI_CONFIG_H" is not defined, evaluates to 0 [-Wundef]
 #if DONT_HAVE_FRIBIDI_CONFIG_H+0
     ^~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /usr/include/fribidi/fribidi.h:35:0,
                 from ../pango/pango-bidi-type.c:44:
/usr/include/fribidi/fribidi-common.h:45:5: warning: "HAVE_FRIBIDI_CUSTOM_H" is not defined, evaluates to 0 [-Wundef]
 #if HAVE_FRIBIDI_CUSTOM_H+0
     ^~~~~~~~~~~~~~~~~~~~~
In file included from /usr/include/fribidi/fribidi-unicode.h:38:0,
                 from /usr/include/fribidi/fribidi.h:37,
                 from ../pango/pango-bidi-type.c:44:
/usr/include/fribidi/fribidi-types.h:84:6: warning: "SIZEOF_WCHAR_T" is not defined, evaluates to 0 [-Wundef]
 # if SIZEOF_WCHAR_T >= 4
      ^~~~~~~~~~~~~~
/usr/include/fribidi/fribidi-types.h:101:5: warning: "FRIBIDI_INT_TYPES" is not defined, evaluates to 0 [-Wundef]
 #if FRIBIDI_INT_TYPES+0
     ^~~~~~~~~~~~~~~~~
In file included from /usr/include/fribidi/fribidi.h:37:0,
                 from ../pango/pango-bidi-type.c:44:
/usr/include/fribidi/fribidi-unicode.h:46:5: warning: "DONT_HAVE_FRIBIDI_UNICODE_VERSION_H" is not defined, evaluates to 0 [-Wundef]
 #if DONT_HAVE_FRIBIDI_UNICODE_VERSION_H+0
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ../pango/pango-bidi-type.c:44:0:
/usr/include/fribidi/fribidi.h:54:5: warning: "FRIBIDI_NO_DEPRECATED" is not defined, evaluates to 0 [-Wundef]
 #if FRIBIDI_NO_DEPRECATED+0
     ^~~~~~~~~~~~~~~~~~~~~

which is https://bugs.freedesktop.org/show_bug.cgi?id=82548. And that is an upstream problem, not a Debian problem. pango simply cannot use -Werror here until it fixes the headers of all its dependencies.

I'm going to push Christoph's workaround as an interim solution to get our build working. Once the fix for *both* of these issues make their way into Debian, then we can update our BuildStream base system, and then remove this hack. (Currently, the base system is based off of Debian testing, but we can consider changing it to sid if need be.)

To be clear, the things that need to happen before pango can turn on -Werror=undef again:

 * Debian's freetype package needs to be fixed
 * Upstream fribidi package needs to be fixed, and that fix needs to make its way into Debian
 * gnome-build-meta needs to switch to using a new ostree system based on the newer Debian

Lastly, I'll note that in WebKit, we do not use -I to include system headers, we use -isystem, to avoid exactly this problem. I don't believe it is reasonable or practical to expect upstream projects to conform to the compiler warnings of downstream projects. Unfortunately the -I is actually coming from pkg-config, so it's not clear to me how it would even be possible to change this.

I'd like to point out that -Werror=undef caught real actual bugs in upstream projects, so I fully expect the change that dropped the warning in Pango to be reverted at the end of the 3.27 cycle.

Ignoring errors is never a good long term strategy.

> we do not use -I to include system headers, we use -isystem, to avoid exactly this problem

Yeah, that's a bad thing to do, and you should fix the projects you're depending on, instead of just blithely ignoring the problem in the hope it goes away. Fun fact: if you use -isystem to include GLib headers, you don't get warnings for misusing g_return_if_fail/g_return_val_if_fail macros.

This reminds me of this “rant” from Behdad about using -Werror by default (that I totally agree with) https://mail.gnome.org/archives/desktop-devel-list/2013-February/msg00093.html

(In reply to Emmanuele Bassi (:ebassi) from comment #9)
> Yeah, that's a bad thing to do, and you should fix the projects you're
> depending on, instead of just blithely ignoring the problem in the hope it
> goes away. Fun fact: if you use -isystem to include GLib headers, you don't
> get warnings for misusing g_return_if_fail/g_return_val_if_fail macros.

We're going to have to disagree. If you insist that the header files of all the projects you depend on must compile cleanly using the set of warnings you've chosen for your own project, you're going to limit yourself in your ability to enable new warnings that would be interesting for your project, but which would cause warnings in the headers of your dependencies. It would not be reasonable to expect projects like glib to ensure their headers compile cleanly using all many possible build warnings that dependent projects choose to enable.

In the case of freetype, yes it was a real bug. In the case of freebidi, SIZEOF_WCHAR_T looks like a bug, while the rest look like false positives. I like -Wundef myself and would prefer to just fix the issues, but if upstream were to close this WONTFIX due to not using -Wundef upstream, that would not be unreasonable.

When I rewrote FriBidi 15 years ago, I was naive and wanted to make the world a better place by making -DXYZ=0 work the same as -UXYZ. That's why I used "#if XYZ+0" instead of "#ifdef XYZ".  I don't remember why I added the +0. I think I hit a gcc bug without it. I can dig.

Anyway, the world seems to have agreed on "#ifdef XYZ" being the pattern. So FriBidi should just be changed to do that. Send a patch to fribidi on github and we'll merge quickly.

Here's an old upstream bug: https://bugs.freedesktop.org/show_bug.cgi?id=82548

(In reply to Behdad Esfahbod from comment #12)
> Anyway, the world seems to have agreed on "#ifdef XYZ" being the pattern. So
> FriBidi should just be changed to do that. Send a patch to fribidi on github
> and we'll merge quickly.

https://github.com/fribidi/fribidi/pull/53, though I think these were harmless, the ones triggering the warning were fixed in the 1.0 release already.

I'm honestly not sure what kind of bugs -Werror=undef catches. "#ifdef TYPO" passes silently as well...

(In reply to Behdad Esfahbod from comment #15)
> I'm honestly not sure what kind of bugs -Werror=undef catches. "#ifdef TYPO"
> passes silently as well...

It has to, otherwise it would be impossible to use #ifdef :P

freetype has been fixed in debian sid: https://tracker.debian.org/news/932653

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/pango/issues/295.