harfbuzz @2.5.1 does not build on PPC Leopard, Mac OS X 10.5.8, because some warnings being treated as errors

[ballapete (Peter "Pete" Dyballa)]

/bin/sh ../libtool  --tag=CXX   --mode=compile /opt/local/bin/g++-mp-6 -DHAVE_CONFIG_H -I. -I..  -pthread -I/opt/local/include/glib-2.0 -I/opt/local/lib/glib-2.0/include -I/opt/local/include/freetype2 -I/opt/local/include/libpng16       -I/opt/local/include  -fno-rtti -pipe -Os -D_GLIBCXX_USE_CXX11_ABI=0 -m32 -fno-exceptions -fno-threadsafe-statics -fvisibility-inlines-hidden -MT libharfbuzz_la-hb-coretext.lo -MD -MP -MF .deps/libharfbuzz_la-hb-coretext.Tpo -c -o libharfbuzz_la-hb-coretext.lo `test -f 'hb-coretext.cc' || echo './'`hb-coretext.cc
libtool: compile:  /opt/local/bin/g++-mp-6 -DHAVE_CONFIG_H -I. -I.. -pthread -I/opt/local/include/glib-2.0 -I/opt/local/lib/glib-2.0/include -I/opt/local/include/freetype2 -I/opt/local/include/libpng16 -I/opt/local/include -fno-rtti -pipe -Os -D_GLIBCXX_USE_CXX11_ABI=0 -m32 -fno-exceptions -fno-threadsafe-statics -fvisibility-inlines-hidden -MT libharfbuzz_la-hb-coretext.lo -MD -MP -MF .deps/libharfbuzz_la-hb-coretext.Tpo -c hb-coretext.cc  -fno-common -DPIC -o .libs/libharfbuzz_la-hb-coretext.o
hb-coretext.cc: In function 'float coretext_font_size_to_ptem(CGFloat)':
hb-coretext.cc:63:17: error: implicit conversion from 'CGFloat {aka float}' to 'double' to match other operand of binary expression [-Werror=double-promotion]
   size *= 72. / 96.;
                 ^~~
hb-coretext.cc: In function 'const __CTFont* create_ct_font(CGFontRef, CGFloat)':
hb-coretext.cc:208:29: warning: the address of 'uint32_t CTGetCoreTextVersion()' will never be NULL [-Waddress]
   if (&CTGetCoreTextVersion != nullptr && CTGetCoreTextVersion() < 0x00070000) {
       ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~
hb-coretext.cc: In function 'hb_bool_t _hb_coretext_shape(hb_shape_plan_t*, hb_font_t*, hb_buffer_t*, const hb_feature_t*, unsigned int)':
hb-coretext.cc:828:64: error: implicit conversion from 'CGFloat {aka float}' to 'double' to match other operand of binary expression [-Werror=double-promotion]
       advances_so_far -= CTLineGetTrailingWhitespaceWidth (line);
                                                                ^
cc1plus: some warnings being treated as errors
make[4]: *** [libharfbuzz_la-hb-coretext.lo] Error 1
make[4]: Leaving directory `/opt/local/var/macports/build/_opt_local_var_macports_sources_nue.de.rsync.macports.org_macports_release_tarballs_ports_graphics_harfbuzz/harfbuzz/work/harfbuzz-2.5.1/src'
make[3]: *** [all-recursive] Error 1
make[3]: Leaving directory `/opt/local/var/macports/build/_opt_local_var_macports_sources_nue.de.rsync.macports.org_macports_release_tarballs_ports_graphics_harfbuzz/harfbuzz/work/harfbuzz-2.5.1/src'
make[2]: *** [all] Error 2
make[2]: Leaving directory `/opt/local/var/macports/build/_opt_local_var_macports_sources_nue.de.rsync.macports.org_macports_release_tarballs_ports_graphics_harfbuzz/harfbuzz/work/harfbuzz-2.5.1/src'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/opt/local/var/macports/build/_opt_local_var_macports_sources_nue.de.rsync.macports.org_macports_release_tarballs_ports_graphics_harfbuzz/harfbuzz/work/harfbuzz-2.5.1'
make: *** [all] Error 2
make: Leaving directory `/opt/local/var/macports/build/_opt_local_var_macports_sources_nue.de.rsync.macports.org_macports_release_tarballs_ports_graphics_harfbuzz/harfbuzz/work/harfbuzz-2.5.1'
Command failed:  cd "/opt/local/var/macports/build/_opt_local_var_macports_sources_nue.de.rsync.macports.org_macports_release_tarballs_ports_graphics_harfbuzz/harfbuzz/work/harfbuzz-2.5.1" && /usr/bin/make -w all 
Exit code: 2

[ballapete (Peter "Pete" Dyballa)]

Main.log from PPC Leopard

[ryandesign (Ryan Carsten Schmidt)]

I ​already patched it to fix all the CGFloat conversion problems I saw that prevented the build on 10.6 i386. I guess it needs a few more similar fixes for 10.5.

[ballapete (Peter "Pete" Dyballa)]

This little patch seems to fix the problem of computing 96.f / 72.f;. I hope to find all places that need a correction.

[ballapete (Peter "Pete" Dyballa)]

Fix computation of 96.f / 72.f

[ballapete (Peter "Pete" Dyballa)]

This failure,

hb-coretext.cc: In function 'hb_bool_t _hb_coretext_shape(hb_shape_plan_t*, hb_font_t*, hb_buffer_t*, const hb_feature_t*, unsigned int)':
hb-coretext.cc:828:64: error: implicit conversion from 'CGFloat {aka float}' to 'double' to match other operand of binary expression [-Werror=double-promotion]
       advances_so_far -= CTLineGetTrailingWhitespaceWidth (line);
                                                                ^

seems to be created by your patch.

The function CTLineGetTrailingWhitespaceWidth() is declared in

# 329 "/System/Library/Frameworks/ApplicationServices.framework/Frameworks/CoreText.framework/Headers/CTLine.h" 3
double CTLineGetTrailingWhitespaceWidth(
 CTLineRef line ) ;

so advances_so_far cannot be typecast as a float.

I am going to patch your patch…

[ryandesign (Ryan Carsten Schmidt)]

Yes, I assumed while developing my patch that it would be a problem that CTLineGetTrailingWhitespaceWidth returns a double, but it compiled on High Sierra so I committed it. The compiler on Leopard is apparently less tolerant of this.

[ballapete (Peter "Pete" Dyballa)]

run_advance needs to stay double as well, because: advances_so_far += run_advance;.

[ballapete (Peter "Pete" Dyballa)]

I think I've found the minimal patch set for Leopard – harfbuzz already built one. I am going to unite (or unify?) the remainder of your patch and mine. And I'll check the changed lines in the source file!

[ballapete (Peter "Pete" Dyballa)]

These are the lines on which the patched variables are used:

  47:#define HB_CORETEXT_DEFAULT_FONT_SIZE 12.f
  50:coretext_font_size_from_ptem (float ptem)
  57:  ptem *= ((CGFloat) 96.f) / ((CGFloat) 72.f);						«===
  58:  return (CGFloat) (ptem <= 0.f ? HB_CORETEXT_DEFAULT_FONT_SIZE : ptem);
  61:coretext_font_size_to_ptem (CGFloat size)
  63:  size *= ((CGFloat) 72.) / ((CGFloat) 96.);						«===
  64:  return size <= 0 ? 0 : size;
 126:release_data (void *info, const void *data, size_t size)
 128:  assert (hb_blob_get_length ((hb_blob_t *) info) == size &&
 320:  CTFontRef ct_font = create_ct_font (cg_font, coretext_font_size_from_ptem (font->ptem));
 344:  if (hb_CGFloat_abs (CTFontGetSize((CTFontRef) data) - coretext_font_size_from_ptem (font->ptem)) > (CGFloat) .5)	«===
 820:    double advances_so_far = 0;
 828:      advances_so_far -= CTLineGetTrailingWhitespaceWidth (line);
 830:	  advances_so_far = -advances_so_far;
 842:      double run_advance = CTRunGetTypographicBounds (run, range_all, nullptr, nullptr, nullptr);
 844:	  run_advance = -run_advance;
 845:      DEBUG_MSG (CORETEXT, run, "Run advance: %g", run_advance);
 921:	  hb_position_t advance = x_advance + y_advance;
 943:	      info->mask = advance;
 952:	  advances_so_far += run_advance;
1021:	  hb_position_t x_offset = (positions[0].x - ((CGFloat) advances_so_far)) * x_mult;	«===
1024:	    CGFloat advance;									«===
1026:	      advance = positions[j + 1].x - positions[j].x;
1028:	      advance = ((CGFloat) run_advance) - (positions[j].x - positions[0].x);		«===
1029:	    info->mask = advance * x_mult;
1037:	  hb_position_t y_offset = (positions[0].y - ((CGFloat) advances_so_far)) * y_mult;	«===
1040:	    CGFloat advance;									«===
1042:	      advance = positions[j + 1].y - positions[j].y;
1044:	      advance = ((CGFloat) run_advance) - (positions[j].y - positions[0].y);		«===
1045:	    info->mask = advance * y_mult;
1052:	advances_so_far += run_advance;

Harfbuzz built, harfbuzz-icu has built too.

[ballapete (Peter "Pete" Dyballa)]

Leopard's patch for CGFloat

[ryandesign (Ryan Carsten Schmidt)]

In 2c5a81262150218255a0dbef5d5ea30174a9d8a4/macports-ports (master):

harfbuzz: Disable -Werror

Closes: #58565
See: #58552

[mascguy (Christopher Nielsen)]

Replying to ryandesign:

In 2c5a81262150218255a0dbef5d5ea30174a9d8a4/macports-ports (master):

harfbuzz: Disable -Werror

Closes: #58565
See: #58552

Since we have an open PR for harfbuzz, just want to check on this: Did this commit alone resolve the issue? Or are the earlier patch(es) necessary as well?

On a related note, harfbuzz-devel has been updated to the latest upstream release. Peter, can you test whether that port builds successfully on 10.5 PPC?

[ballapete (Peter "Pete" Dyballa)]

Replying to mascguy:

On a related note, harfbuzz-devel has been updated to the latest upstream release. Peter, can you test whether that port builds successfully on 10.5 PPC?

I think version 2.8.2 built here without my patches in July. I am preparing to build versions 2.9.0.

[ballapete (Peter "Pete" Dyballa)]

Replying to mascguy:

On a related note, harfbuzz-devel has been updated to the latest upstream release. Peter, can you test whether that port builds successfully on 10.5 PPC?

harfbuzz @2.9.0 built and installed without problem. The compiler used is GCC7.

[ballapete (Peter "Pete" Dyballa)]

Replying to mascguy:

harfbuzz-devel @2.9.0 built as well without problem and GCC7.

[mascguy (Christopher Nielsen)]

Replying to ballapete:

harfbuzz-devel @2.9.0 built as well without problem and GCC7.

Beautiful, glad it's working now. And thanks for the update!