[Bug 211201] print/freetype2: update to 2.7, add V40 code

[bugzilla-noreply at freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=211201

--- Comment #55 from Jan Beich (mail not working) <jbeich at FreeBSD.org> ---
Comment on attachment 175507
  --> https://bugs.freebsd.org/bugzilla/attachment.cgi?id=175507
Proposed patch (since 412348 revision)

Resuming review. I'm uneasy to land this per timeout due to the following style
issues:

> +SUBPIXEL_HINTING_DESC=	Sub-pixel hinting support
> +V38_DESC=	v38 mode (Infinality code)
> +V38_VARS=	SUBPIXEL_HINTING_MODE+=1
> +V40_DESC=	v40 mode (minimal code, a.k.a. ClearType hinting)
> +V40_VARS=	SUBPIXEL_HINTING_MODE+=2
> +
> +.include <bsd.port.pre.mk>
> +
> +SELECTED_MODE=	r=0; for m in ${SUBPIXEL_HINTING_MODE};\
> +	do r=$$(($$r | $$m)); done; ${ECHO_CMD} $$r
> [...]
> +post-patch:
> +.if defined(SUBPIXEL_HINTING_MODE)
> +	@${REINPLACE_CMD} -e \
> +		's|^\(#define TT_CONFIG_OPTION_SUBPIXEL_HINTING\).*|\1 \
> +		${SELECTED_MODE:sh}|' \
> +		${WRKSRC}/include/freetype/config/ftoption.h
> +.else
> +	@${REINPLACE_CMD} -e \
> +		's|^\(#define TT_CONFIG_OPTION_SUBPIXEL_HINTING.*\)|/* \1 */|' \
> +		${WRKSRC}/include/freetype/config/ftoption.h
> +.endif

Can we avoid the complexity in this case? It'd even be 4 lines shorter.

  SUBPIXEL_HINTING_DESC=        Sub-pixel hinting support
  V38_DESC=     v38 mode (Infinality code)
  V40_DESC=     v40 mode (minimal code, a.k.a. ClearType hinting)

  .include <bsd.port.options.mk>

  .if ${PORT_OPTIONS:MV38} && ${PORT_OPTIONS:MV40}
  CFLAGS+=      -DTT_CONFIG_OPTION_SUBPIXEL_HINTING=3
  .elif ${PORT_OPTIONS:MV40}
  CFLAGS+=      -DTT_CONFIG_OPTION_SUBPIXEL_HINTING=2
  .elif ${PORT_OPTIONS:MV38}
  CFLAGS+=      -DTT_CONFIG_OPTION_SUBPIXEL_HINTING=1
  .endif

  post-patch:
  # warning: 'TT_CONFIG_OPTION_SUBPIXEL_HINTING' macro redefined
[-Wmacro-redefined]
          @${REINPLACE_CMD} -e '/^#define TT_CONFIG_OPTION_SUBPIXEL_HINTING/d'
\
                  ${WRKSRC}/include/freetype/config/ftoption.h

> +post-patch-DEBUG-on:
> +	@${SED} -i '.d.bak' -e 's|.*\(#define FT_DEBUG_LEVEL_TRACE\).*|\1| ; \
> +		s|.*\(#define FT_DEBUG_MEMORY\).*|\1|' \
> +		${WRKSRC}/include/freetype/config/ftoption.h
> +

- Use "-e" option to split commands rather than ";" to avoid tracking open
quotes
- Don't use SED when modifying in-place
- Make backup suffix a bit more verbose than just one confusing letter

Here's what I have in mind:

  post-patch-DEBUG-on:
          @${REINPLACE_CMD} -i '.bak.DEBUG' \
                  -e 's/.*\(#define FT_DEBUG_LEVEL_TRACE\).*/\1/' \
                  -e 's/.*\(#define FT_DEBUG_MEMORY\).*/\1/' \
                  ${WRKSRC}/include/freetype/config/ftoption.h

-- 
You are receiving this mail because:
You are on the CC list for the bug.