[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-2d-dev
Subject: Re: [OpenJDK 2D-Dev] [PATCH FOR REVIEW] fix for bug 8011693: Remove redundant fontconfig files
From: Andrew Hughes <gnu.andrew () redhat ! com>
Date: 2013-05-30 16:42:06
Message-ID: 915308066.1099648.1369932126532.JavaMail.root () redhat ! com
[Download RAW message or body]
----- Original Message -----
> On 05/30/2013 03:29 PM, Andrew Hughes wrote:
> > ----- Original Message -----
> > > > None of these are touched any more in the current patch, as we don't
> > > > build
> > > > on
> > > > these targets.
> > >
> > > Right, I noted that in the first sentence of my prior email.
> > >
> > > > we haven't tested on *BSD.
> > >
> > > fair enough but the difference is that there is no build
> > > line that ever copies this file, so its un-used.
> > >
> >
> > Yes, it was added in:
> >
> > changeset: 5117:d45bc4307996
> > user: michaelm
> > date: Tue Mar 06 20:34:38 2012 +0000
> > summary: 7113349: Initial changeset for Macosx port to jdk
> >
> > That patch doesn't make any Makefile changes to add it in, but it may still
> > be used
> > by *BSD users as they have other patches not yet upstreamed. I'd rather
> > they made
> > the decision as to whether or not it should be removed.
>
> Ok. So I have kept it in.
> >
> > > > From my review of the patch, I think the variables are left empty, not
> > > > removed:
> > >
> > > Looking at
> > > http://jvanek.fedorapeople.org/oracle/jdk8/webrevs/removedFontConfigFiles-linuxOnly/makefiles/GendataFontConfig.gmk.sdiff.html
> > >
> > > they seem to be removed.
> > >
> > > I was expecting to see
> > > GENDATA_FONT_CONFIG_SRC_DIR := \
> > > 38 $(JDK_TOPDIR)/src/solaris/classes/sun/awt/fontconfigs
> > >
> > > may be left in there and also that it left just
> > >
> > > GENDATA_FONT_CONFIG_SRC_FILES :=
> > >
> > > rather than have it removed completely
> > >
> >
>
> Ok. I have added them as you have suggested
>
> http://jvanek.fedorapeople.org/oracle/jdk8/webrevs/removedFontConfigFiles-linuxOnly_3/
>
> > Ok I was thinking of the 7 makefile:
> >
> > -FONTCONFIGS_SRC = $(PLATFORM_SRC)/classes/sun/awt/fontconfigs
> > -_FONTCONFIGS = \
> > - fontconfig.properties \
> > - fontconfig.SuSE.properties \
> > - fontconfig.Ubuntu.properties \
> > - fontconfig.Fedora.properties
> > +FONTCONFIGS_SRC =
> > +_FONTCONFIGS =
> > +
> >
> > When are these finally going to be removed? It's very confusing.
>
> I'm afraid we can not just take them off as they are widely used.
> I think empty is safer here then remove, as they are kept for mac and win
> port.
Ok, I tested and pushed the new version:
http://hg.openjdk.java.net/jdk8/2d/jdk/rev/fd377533608b
> >
> > > -phil.
> > >
> > >
> > > On 5/29/2013 1:17 PM, Andrew Hughes wrote:
> > > > ----- Original Message -----
> > > > > Jiri,
> > > > >
> > > > > I think this has mostly been hashed out as the fix is reduced to Linux
> > > > > but here's my over-due input :
>
> I agree it have hashed, but fix the underlying windows issue is quite complex
> task. At least it
> deserves another changeset. And especially it deserves windows specialist:)
>
> > > > >
>
> Yuhuuu! This is an feedback I'm happy for :)
>
> It explains so much.....
>
> > > > > 1) Windows *absolutely* still needs fontconfig files.
> > > > >
> > > > > 2) Mac OS X doesn't obey what's there but that doesn't mean its
> > > > > going to work when you just remove them.
>
> Then it should be tested and removed rather then keeping undetermined code.
> > > > >
> > > > > 3) Solaris *does* want the Solaris one, even though it has the
> > > > > same fontconfig platform support as Linux. This is for compatibility.
> > > > >
> > > > None of these are touched any more in the current patch, as we don't
> > > > build
> > > > on
> > > > these targets.
> > > >
> > > > > 4) Linux does not need them *so long as* fontconfig is there and
> > > > > working.
> > > > > OpenJDK on any Linux desktop should be fine.
> > > > >
> > > > > 5) However policy decisions were made to leave them there for some
> > > > > particular linux variants in the closed repo, again for compatibility,
> > > > > but you are leaving that alone, so that's fine, although we should
> > > > > revisit this
> > > > >
> > > > > 6) I never noticed the bsd one before. It must have snuck in with mac
> > > > > port.
> > > > > Is anything even referencing it ? If not I think this can be removed
> > > > > too.
> > > > >
> > > > I don't think this should be part of this patch for the same reason
> > > > Solaris/Mac OS/Windows
> > > > aren't; we haven't tested on *BSD.
>
> agree
> > > >
> > > > > 7) The files themselves and support for the files are distinct issues.
> > > > > There should still be the ability for [say] Gentoo, to decide they want
> > > > > a particular set of fonts used and so they will ship a file .
> > > > > So it might be better to leave the variables there (empty) and with
> > > > > a comment that this is a placeholder.
> > > > We do actually carry one for both Gentoo and RHEL in IcedTea for OpenJDK
> > > > 6.
> > > > From my review of the patch, I think the variables are left empty, not
> > > > removed:
> > > >
> > > > http://jvanek.fedorapeople.org/oracle/jdk8/webrevs/removedFontConfigFiles-linuxOnly/jdk.patch
> > > >
> > > > > -phil.
> > > > >
> > > > > On 5/29/2013 5:06 AM, Andrew Hughes wrote:
> > > > > > ----- Original Message -----
> > > > > > > On 05/20/2013 04:37 PM, Jiri Vanek wrote:
> > > > > > > > On 05/10/2013 04:08 PM, Jiri Vanek wrote:
> > > > > > > > > On 04/08/2013 05:31 PM, Jiri Vanek wrote:
> > > > > > > > > > On 04/08/2013 04:13 PM, Vladislav Karnaukhov wrote:
> > > > > > > > > > > Hello Jiri,
> > > > > > > > > > >
> > > > > > > > > > > please see inline.
> > > > > > > > > > >
> > > > > > > > > > > On 4/8/2013 05:29 PM, Jiri Vanek wrote:
> > > > > > > > > > > > On 04/08/2013 02:39 PM, Vladislav Karnaukhov wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > Thank you very much for win-check! It will force me to \
> > > > > > > > > > > > install new
> > > > > > > > > > > > windows machine somewhere.
> > > > > > > > > > > > Do you mind do check if pure removal of fontconfig files \
> > > > > > > > > > > > (both src
> > > > > > > > > > > > and
> > > > > > > > > > > > bfc) from you installed jdk7/8 on windows will work? \
> > > > > > > > > > > > (should)
> > > > > > > > > > > Yes, I've checked and it does *not* work. That's the reason why \
> > > > > > > > > > > I replied to your very first
> > > > > > > > > > > message. A removal of fontconfig.* files simply crashes Java, - \
> > > > > > > > > > > on both
> > > > > > > > > > > Windows and Mac, - because
> > > > > > > > > > > some font management-related classes rely on these files. Hence \
> > > > > > > > > > > my question regarding deeper
> > > > > > > > > > > re-design on font management system...
> > > > > > > > > > >
> > > > > > > > > > > I've tested Mac build as well, and there's the same error:
> > > > > > > > > > Ok. I will try anyway:)
> > > > > > > > > > For linux I'm quite sure the new fontmanagers are working pretty
> > > > > > > > > > fine.
> > > > > > > > > > Do you think it will be acceptable to prepare smaller clean up - \
> > > > > > > > > > to remove all linux fontconfig
> > > > > > > > > > files?
> > > > > > > > > >
> > > > > > > > > > And later, as separate changeset to fontmanagers for \
> > > > > > > > > > windows/mac, but
> > > > > > > > > > I'm afraid I will not be
> > > > > > > > > > capable of such an development on non linux system.
> > > > > > > > > >
> > > > > > > > > > Thanx for your help,
> > > > > > > > > >
> > > > > > > > > > J.
> > > > > > > > > Hi!
> > > > > > > > >
> > > > > > > > > I had finally found some free time, so here it is - smaller version
> > > > > > > > > which
> > > > > > > > > is removing just stuff for
> > > > > > > > > linux when OpenJDK is defined.
> > > > > > > > >
> > > > > > > > > http://jvanek.fedorapeople.org/oracle/jdk8/webrevs/removedFontConfigFiles-linuxOnly/
> > > > > > > > >
> > > > > > > > > Although I had windows build, I lost this machine so - again (and
> > > > > > > > > sorry
> > > > > > > > > for that) - tested only on
> > > > > > > > > Fedora.
> > > > > > > > >
> > > > > > > > > Also when I read the individual fontmanagers, I believe that they
> > > > > > > > > really
> > > > > > > > > *should* work without
> > > > > > > > > fontocfigs. So although this is fixing the 8011693, new bugs should
> > > > > > > > > be
> > > > > > > > > filled for windows and mac,
> > > > > > > > > because theirs implementations are broken.
> > > > > > > > >
> > > > > > > > > Thank you very much for any comments.
> > > > > > > > >
> > > > > > > > > Best Regards
> > > > > > > > > j.
> > > > > > > Ping?
> > > > > > >
> > > > > > > Any advice how to move this forward?
> > > > > > >
> > > > > > > > I know that this is minor fix compared to others I can read on this
> > > > > > > > channel, but as the font
> > > > > > > > managers exists, and fontconfig files *should* be redundant, then
> > > > > > > > this
> > > > > > > > change should be done. If
> > > > > > > > fontmanagers are buggy (and eg windows one appeared to be) then as
> > > > > > > > soon
> > > > > > > > as
> > > > > > > > this will be tempted then
> > > > > > > > sooner it will get fixed.
> > > > > > > >
> > > > > > > > For linux I'm pretty sure this is working, and we have even removed
> > > > > > > > the
> > > > > > > > fontconfig files from
> > > > > > > > packages in public facing version three months ago [1]
> > > > > > > >
> > > > > > > > So this can be first step to get rid of old and redundant font
> > > > > > > > mapping
> > > > > > > > completely.
> > > > > > > >
> > > > > > > >
> > > > > > > > J.
> > > > > > > >
> > > > > > > >
> > > > > > > > [1]
> > > > > > > > http://pkgs.fedoraproject.org/cgit/java-1.7.0-openjdk.git/commit/?h=f17&id=9d6dd62ae2123635b4d15e40e527a0b617756484
> > > > > > > >
> > > > > > > > (search for +rm %{buildoutputdir}/j2re-image/lib/fontconfig)
> > > > > > I've applied this patch and built OpenJDK, and it went fine. A basic
> > > > > > Swing
> > > > > > application still loads up fine.
>
> Thank you very much for review!
> I was already lost in despair that this rotten files will remain inside.
>
> > > > > >
> > > > > > So looks good to go to me.
> > > > >
> > >
> > >
> >
>
>
--
Andrew :)
Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
PGP Key: 248BDC07 (https://keys.indymedia.org/)
Fingerprint = EC5A 1F5E C0AD 1D15 8F1F 8F91 3B96 A578 248B DC07
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic