Hi Michael,
Am Dienstag, den 04.09.2018, 11:34 +0200 schrieb Michael Olbrich:
> On Tue, Aug 07, 2018 at 02:40:47PM +0200, Florian Bäuerle wrote:
> > As all ttfs font packages, xorg-font-ttf-android used to install fonts
> > into xorg-fonts' package and defers the mkfontdir- and mkfontscale steps
> > to xorg-fonts.targetinstall. This was a nice trick to spare the
> > generation of fonts.{dir,scale} in every font package, but it breaks the
> > collection feature for ttf-fonts.
> >
> > Signed-off-by: Florian Bäuerle <[email protected]>
> > ---
> > rules/xorg-font-ttf-android.make | 50 ++++++++++++++++++++------------
> > 1 file changed, 32 insertions(+), 18 deletions(-)
> >
> > diff --git a/rules/xorg-font-ttf-android.make
> > b/rules/xorg-font-ttf-android.make
> > index 443f855aa..adad187c5 100644
> > --- a/rules/xorg-font-ttf-android.make
> > +++ b/rules/xorg-font-ttf-android.make
> > @@ -1,6 +1,7 @@
> > # -*-makefile-*-
> > #
> > # Copyright (C) 2015 by Michael Olbrich <[email protected]>
> > +# (C) 2018 by Florian Bäuerle <[email protected]>
> > #
> > # See CREDITS for details about who has contributed to this project.
> > #
> > @@ -28,18 +29,8 @@ XORG_FONT_TTF_ANDROID_LICENSE := Apache-2.0
> > XORG_FONT_TTF_ANDROID_LICENSE_FILES := \
> > file://NOTICE;md5=9645f39e9db895a4aa6e02cb57294595
> >
> > -ifdef PTXCONF_XORG_FONT_TTF_ANDROID
> > -$(STATEDIR)/xorg-fonts.targetinstall.post:
> > $(STATEDIR)/xorg-font-ttf-android.targetinstall
> > -endif
> > -
> > -#
> > ----------------------------------------------------------------------------
> > -# Prepare
> > -#
> > ----------------------------------------------------------------------------
> > -
> > -#
> > -# autoconf
> > -#
> > -XORG_FONT_TTF_ANDROID_CONF_TOOL := NO
>
> Don't remove this.
>
> > +XORG_FONT_TTF_ANDROID_FONTDIR :=
> > $(XORG_FONTDIR)/truetype/android
>
> ok
>
> > +XORG_FONT_TTF_ANDROID_FONTPKGDIR :=
> > $(XORG_FONT_TTF_ANDROID_PKGDIR)$(XORG_FONT_TTF_ANDROID_FONTDIR)
>
> remove
>
> >
> > #
> > ----------------------------------------------------------------------------
> > # Compile
> > @@ -55,6 +46,18 @@ $(STATEDIR)/xorg-font-ttf-android.compile:
> >
> > $(STATEDIR)/xorg-font-ttf-android.install:
> > @$(call targetinfo)
> > +
> > + @rm -rf $(XORG_FONT_TTF_ANDROID_PKGDIR)
> > + @mkdir -p $(XORG_FONT_TTF_ANDROID_FONTPKGDIR)
> > + @find $(XORG_FONT_TTF_ANDROID_DIR) \
> > + -name "*.ttf" \
> > + | \
> > + while read file; do \
> > + install -m 644 $${file} $(XORG_FONT_TTF_ANDROID_FONTPKGDIR); \
> > + done
> > + @mkfontscale $(XORG_FONT_TTF_ANDROID_FONTPKGDIR)
> > + @mkfontdir $(XORG_FONT_TTF_ANDROID_FONTPKGDIR)
>
> This adds a lot of boiler plate for all packages. I'd like to see this here:
>
> @$(call world/install-fonts, XORG_FONT_TTF_ANDROID, *.ttf)
I would have also preferred that, *but* if you look at xorg-font-ttf-noto, the
original package author did not want to just install every font present in the
archive. I did not question that decision and instead concluded, that such a
macro cannot easily do the job for every font package.
So should I
a) reconsider the original author's decision
or
b) add the boilerplate to the noto package and use the macro everywhere else
I know the current approach sucks, so much that I edited all the makefiles with
sed.
>
> And in rules/post/ptxd_make_world_fonts.make
>
> define world/install-fonts
> rm -rf $($(1)_PKGDIR)
> ...
> @mkfontdir $($(1)_PKGDIR)/$($(1)_FONTDIR)
> endef
>
>
>
> > +
> > @$(call touch)
> >
> > #
> > ----------------------------------------------------------------------------
> > @@ -64,14 +67,25 @@ $(STATEDIR)/xorg-font-ttf-android.install:
> > $(STATEDIR)/xorg-font-ttf-android.targetinstall:
> > @$(call targetinfo)
> >
> > - @mkdir -p $(XORG_FONTS_DIR_INSTALL)/truetype
> > + @$(call install_init, xorg-font-ttf-android)
> > + @$(call install_fixup, xorg-font-ttf-android,PRIORITY,optional)
> > + @$(call install_fixup, xorg-font-ttf-android,SECTION,base)
> > + @$(call install_fixup, xorg-font-ttf-android,AUTHOR,"Florian Bäuerle
> > <[email protected]>")
> > + @$(call install_fixup, xorg-font-ttf-android,DESCRIPTION,missing)
> >
> > - @find $(XORG_FONT_TTF_ANDROID_DIR) \
> > - -name "*.ttf" \
> > - | \
> > - while read file; do \
> > - install -m 644 $${file} $(XORG_FONTS_DIR_INSTALL)/truetype; \
> > + @$(call install_tree, xorg-font-ttf-android, 0, 0, -,
> > $(XORG_FONT_TTF_ANDROID_FONTDIR))
> > +
> > +ifdef PTXCONF_XORG_FONTS_QT4_LINKS
> > + @cd $(XORG_FONT_TTF_ANDROID_PKGDIR) && \
> > + find . -type f -name "*.ttf" | while read file; do \
> > + name=$$(basename $$file); \
> > + $(call install_link, xorg-font-ttf-android, \
> > + ../../..$(XORG_FONT_TTF_ANDROID_FONTDIR)/$$name, \
> > + /usr/lib/fonts/$$name); \
> > done
> > +endif
>
> Create the symlinks in _PKGDIR in world/install-fonts and use '/' in
> install_tree above. You'll need to us a shell 'if' instead of 'ifdef' in
> world/install-fonts:
>
> if [ "$(PTXCONF_XORG_FONTS_QT4_LINKS)" = "y" ]; then \
> ...
>
>
> The current code and your copies in all the different rules ignore some
> errors. I want to clean this up at some point and that will be easier if I
> only have to modify one or two copies and not all font rules.
>
> These comments apply to all other patches as well.
>
Thanks for the review!
- Florian
_______________________________________________
ptxdist mailing list
[email protected]
