[OpenJDK 2D-Dev] [PATCH FOR REVIEW] fix for bug 8011693: Remove redundant fontconfig files

Andrew Hughes gnu.andrew at redhat.com
Thu May 30 16:42:06 UTC 2013 


----- 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

More information about the 2d-dev mailing list