Building against libstdc++

[Miklos Vajna]

Hi,

We (libreoffice) build pdfium against libstdc++ with gcc on Linux.
Currently we have a downstream patch to fix up the build for that
use-case:

https://cgit.freedesktop.org/libreoffice/core/tree/external/pdfium/build.patch.1

The majority of the patch is missing string.h includes (8 in total),
which I think happens because the pdfium trybots build against libc++
instead.

Is there interest in libstdc++ build fixes upstream or should we keep
these tweaks on our side?

I'm happy to submit the missing includes to gerrit if they are welcome.

Thanks,

Miklos

[Lei Zhang]

Taking a quick look, most of that patch looks good for upstreaming,
except removing constexpr from third_party/base/span.h. That is likely
an issue with GCC. In the long run, when C++20 usage is allowed, we'll
just switch to std::span which has a constexpr ctor.

In the bug tracker, we have https://crbug.com/pdfium/956 open for
having continuous testing with GCC. I do build with GCC locally every
so often and fix compile errors as they come up. One open question
here is what version of GCC to support. Which version does LibreOffice
use?

> --
> You received this message because you are subscribed to the Google Groups "pdfium" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to pdfium+un...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/pdfium/YPadZpgxhe4nXucY%40vmiklos.hu.

[Miklos Vajna]

On Tue, Jul 20, 2021 at 09:50:13AM -0700, Lei Zhang <the...@chromium.org> wrote:
> Taking a quick look, most of that patch looks good for upstreaming,
> except removing constexpr from third_party/base/span.h. That is likely
> an issue with GCC. In the long run, when C++20 usage is allowed, we'll
> just switch to std::span which has a constexpr ctor.

Sure, I only focused on the missing includes, the rest is a random
collection of local workarounds to please our older baseline. :-)

I've put <https://pdfium-review.googlesource.com/c/pdfium/+/83210> up
for review regarding the includes.

> In the bug tracker, we have https://crbug.com/pdfium/956 open for
> having continuous testing with GCC. I do build with GCC locally every
> so often and fix compile errors as they come up. One open question
> here is what version of GCC to support. Which version does LibreOffice
> use?

GCC 7.0.0 is the libreoffice baseline (see
<https://cgit.freedesktop.org/libreoffice/core/tree/README.md#n45>), but
in general, it's not my intention to force this baseline on any
dependency we bundle. It's just this longer list of missing includes is
something that can be potentially problematic to maintain downstream,
and looked like an "it worked by accident before" situation, so good to
fix.

[Lei Zhang]

On Wed, Jul 21, 2021 at 1:18 AM Miklos Vajna <vmi...@vmiklos.hu> wrote:
>
> On Tue, Jul 20, 2021 at 09:50:13AM -0700, Lei Zhang <the...@chromium.org> wrote:
> > Taking a quick look, most of that patch looks good for upstreaming,
> > except removing constexpr from third_party/base/span.h. That is likely
> > an issue with GCC. In the long run, when C++20 usage is allowed, we'll
> > just switch to std::span which has a constexpr ctor.
>
> Sure, I only focused on the missing includes, the rest is a random
> collection of local workarounds to please our older baseline. :-)
>
> I've put <https://pdfium-review.googlesource.com/c/pdfium/+/83210> up
> for review regarding the includes.

Thanks. That's in the CQ. I think it is mostly because the files in
question use memset(). For my local GCC build, I have
use_custom_libcxx=false set in my GN args, but for some reason I don't
see the same compile errors.

> > In the bug tracker, we have https://crbug.com/pdfium/956 open for
> > having continuous testing with GCC. I do build with GCC locally every
> > so often and fix compile errors as they come up. One open question
> > here is what version of GCC to support. Which version does LibreOffice
> > use?
>
> GCC 7.0.0 is the libreoffice baseline (see
> <https://cgit.freedesktop.org/libreoffice/core/tree/README.md#n45>), but
> in general, it's not my intention to force this baseline on any
> dependency we bundle. It's just this longer list of missing includes is
> something that can be potentially problematic to maintain downstream,
> and looked like an "it worked by accident before" situation, so good to
> fix.

Good to know. Locally I have GCC 8.x, 9.x, and 10.x, with 10.x being
the default and the one I test with.