harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Hindess <mark.hind...@googlemail.com>
Subject Re: [classlib] Trying to catch patternset errors earlier
Date Thu, 05 Oct 2006 14:22:10 GMT

On 5 October 2006 at 10:05, "Geir Magnusson Jr." <geir@pobox.com> wrote:
> Before you do that... you would be putting similar information in the 
> build.xml file?  Or am I misunderstanding something?  having it in the 
> patternset does make it easy to find stuff :)

I'll be putting something in the build.xml file ... the ant from the
accessibility example below.  That is a patternset that just figures
everything out from the source tree.

Essentially it's just saying include build/classes/blah if:

  1) blah is also in the source tree - a resource file for example
  2) there is a corresponding blah.java file in the source tree

I've never really looked in the patternset files when trying to find
something.  I'd just look in the src/main/java tree or the jar table of
contents.  What do you use the patternsets for?

Regards,
 Mark.


> Mark Hindess wrote:
> > Ok.  There haven't been any shouts against it so.  I'm going to split
> > the .java files that contain two classes and then dump the patternsets.
> > 
> > Regards,
> >  Mark.
> > 
> > On 3 October 2006 at 11:27, Oliver Deakin <oliver.deakin@googlemail.com> wr
> ote:
> >> Mark Hindess wrote:
> >>> On 28 September 2006 at 14:58, "Alexey Petrenko" <alexey.a.petrenko@gmail
> .c
> >> om> wrote:
> >>>   
> >>>> 2006/9/28, Mark Hindess <mark.hindess@googlemail.com>:
> >>>>     
> >>>>> On 28 September 2006 at 14:30, "Alexey Petrenko" <alexey.a.petrenko@gma
> il
> >> .c
> >>>>>       
> >>>> om> wrote:
> >>>>     
> >>>>>> I think that it will be better to add another target to build
for
> >>>>>> this check.
> >>>>>> Because of two reasons:
> >>>>>> 1. It is unclear that clean is also checks something
> >>>>>>         
> >>>>> This simple check can only happen part way through the clean target
-
> >>>>> after the modules have done their cleaning and before the top-level
> >>>>> cleanup is done.
> >>>>>       
> >>>> I see your point.
> >>>>
> >>>> But let's review the sitatuation from another viewpoint...
> >>>> What do we need in general?
> >>>> We need up-to-date list of all packages in the specified module. Right?
> >>>> If so we can create simple script to update patternset files... Or
> >>>> even create them at build time. But will cost us build time itself :)
> >>>>     
> >>> That's a reasonable point.  I had a look at what it would take to get
> >>> rid of the patternsets completely.  It's quite easy for some modules.
> >>> For example, for accessibility, we can just replace the "classes" 
> >>> fileset with:
> >>>
> >>>      <fileset id="classes" dir="${hy.build}">
> >>>         <or>
> >>>             <present targetdir="${hy.accessibility.src.main.java}" />
> >>>             <present targetdir="${hy.accessibility.src.main.java}">
> >>>                 <mapper type="regexp"
> >>>                         from="^(.*?)(\$$[^/\\\.]*)?\.class$$"
> >>>                         to="\1.java"/>
> >>>             </present>
> >>>         </or>
> >>>     </fileset>
> >>>
> >>> Which basically says include files if they are also present in
> >>> the source directory - typically resource files - or if they are
> >>> .class files and have an obviously-named source file in the source
> >>> directory[0].
> >>>   
> >> +1 to building the patternset on the fly rather than relying on file 
> >> lists being manually
> >> kept up to date. I can't see any reason for keeping the patternset files.
> >>
> >>> This works for most modules.  The present tags are duplicated for
> >>> modules with platform source directories.
> >>>
> >>> However, it doesn't work very well for awt, beans, jndi, luni, and
> >>> security.  Why?  Because they have some source files defining two
> >>> classes.  For instance, java.awt.ContextImpl defined in:
> >>>
> >>>   modules/awt/src/main/java/common/java/awt/Component.java
> >>>
> >>> We could hardcode these special case - there are only a few.  Or perhaps
> >>> we could split them in to distinct source files?
> >>>   
> >> I'd go for splitting them out into separate files - hardcoding them 
> >> means that you're still
> >> keeping a manual file list for those classes. Does anyone have an issue 
> >> with separating
> >> these classes out?
> >>
> >>> I have some patches ready to make this change but I've not committed
> >>> anything.
> >>>
> >>> Personally I think getting rid of the patternsets would be a good idea
> >>> since it reduces the coupling - although really the coupling is only
> >>> strictly necessary for luni and security.
> >>>
> >>> It is important to note that even if we do something like this, leaving
> >>> the catch-all check in the clean target still helps stop problems - if
> >>> people forget to add the hardcoded special cases or if we split them
> >>> people add new source defining two classes in one file.
> >>>   
> >> Agreed - keeping the clean target check helps us make sure that no one 
> >> breaks the
> >> jar packaging in the future. As long as it completes the clean before 
> >> failing, that's
> >> fine.
> >>
> >> Regards,
> >> Oliver
> >>
> >>> Regards,
> >>>  Mark.
> >>>
> >>> [0] The simpler '<mapper type="glob" from="*.class" to="*.java">'
> >>> doesn't work because it misses inner classes.
> >>>
> >>>   
> >>>>>> 2006/9/28, Mark Hindess <mark.hindess@googlemail.com>:
> >>>>>>         
> >>>>>>> On 28 September 2006 at 11:07, Tim Ellison <t.p.ellison@gmail.com>
wr
> ot
> >>>>>>>           
> >>>> e:
> >>>>     
> >>>>>>>> Sounds reasonable.  The alternative is to not make clean
fail, just
> >>>>>>>> print the warning and tidy up the remainder.  That may
be too easy t
> o
> >>>>>>>> ignore though.
> >>>>>>>>             
> >>>>>>> Yes.  I considered that and had the same concern.  Even
if I wrote th
> e
> >>>>>>> code to print the warning, I think I'd ignore it since it
would scrol
> l
> >>>>>>> too quickly off the top of my screen at the beginning of
the build.
> >>>>>>>
> >>>>>>> -Mark.
> >>>>>>>
> >>>>>>>           
> >>>>>>>> Regards,
> >>>>>>>> Tim
> >>>>>>>>
> >>>>>>>> Mark Hindess wrote:
> >>>>>>>>             
> >>>>>>>>> Yesterday, while looking at something unrelated,
I noticed that som
> >>>>>>>>>               
> >>>> e
> >>>>     
> >>>>>>>>> of the patternsets that are used to select the jars
for the classli
> >>>>>>>>>               
> >>>> b
> >>>>     
> >>>>>>>>> modules were not up to date with the result that
some classes would
> >>>>>>>>>               
> >>>>  be
> >>>>     
> >>>>>>>>> missing from the resulting jars[0].
> >>>>>>>>>
> >>>>>>>>> While it makes me slightly uneasy having a clean
target that might 
> >>>>>>>>>               
> >>>> fail
> >>>>     
> >>>>>> ,
> >>>>>>         
> >>>>>>>>> it turns out that this is one place where it is
quite easy to check
> >>>>>>>>> whether the patternsets are out of date.  (Because
if after the mod
> >>>>>>>>>               
> >>>> ules
> >>>>     
> >>>>>>>>> have cleaned classes in their patternsets there
are still files lef
> >>>>>>>>>               
> >>>> t
> >>>>     
> >>>>>>>>> over then something is being create that isn't accounted
for by the
> >>>>>>>>> patternsets.)
> >>>>>>>>>
> >>>>>>>>> So the clean target will now fail with a message
like (tested
> >>>>>>>>> by reverting yesterdays change to the sound patternset):
> >>>>>>>>>
> >>>>>>>>>   Built files still exist after module clean targets
have run.  Thi
> >>>>>>>>>               
> >>>> s
> >>>>     
> >>>>>>>>>   probably means that one or more patternsets are
incomplete.  The
> >>>>>>>>>   remaining files are:
> >>>>>>>>>
> >>>>>>>>>   /classlib/build/classes/org/apache/harmony/sound/utils/ProviderSe
> >>>>>>>>>               
> >>>> rvic
> >>>>     
> >>>>>> e.cl
> >>>>>>         
> >>>>>>>> ass
> >>>>>>>>             
> >>>>>>>>> I'm sure there are other ways to solve this problem
but this seemed
> >>>>>>>>>               
> >>>>  lik
> >>>>     
> >>>>>> e
> >>>>>>         
> >>>>>>>>> a sensible quick fix to help catch these problems
a little sooner[1
> >>>>>>>>>               
> >>>> ].
> >>>>     
> >>>>>>>>> Regards,
> >>>>>>>>>  Mark.
> >>>>>>>>>
> >>>>>>>>> [0] This might explain some of the awt/swing test
failures so perha
> >>>>>>>>>               
> >>>> ps
> >>>>     
> >>>>>>>>> it is worth checking the exclude lists again?
> >>>>>>>>>
> >>>>>>>>> [1] Though I guess since we clean at the beginning
of a build (not 
> >>>>>>>>>               
> >>>> the
> >>>>     
> >>>>>>>>> end) then we might still find them in the build
after the one that
> >>>>>>>>> caused the break but that's better than only finding
them by accide
> >>>>>>>>>               
> >>>> nt.
> >>>>     
> >>>>>>>>> -------------------------------------------------------------------
> >>>>>>>>>               
> >>>> --
> >>>>     
> >>>>>>>>> Terms of use : http://incubator.apache.org/harmony/mailing.html
> >>>>>>>>> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.or
> >>>>>>>>>               
> >>>> g
> >>>>     
> >>>>>>>>> For additional commands, e-mail: harmony-dev-help@incubator.apache.
> >>>>>>>>>               
> >>>> org
> >>>>     
> >>>>>>>>>               
> >>>>>>>> --
> >>>>>>>>
> >>>>>>>> Tim Ellison (t.p.ellison@gmail.com)
> >>>>>>>> IBM Java technology centre, UK.
> >>>>>>>>
> >>>>>>>> --------------------------------------------------------------------
> -
> >>>>>>>> Terms of use : http://incubator.apache.org/harmony/mailing.html
> >>>>>>>> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> >>>>>>>> For additional commands, e-mail: harmony-dev-help@incubator.apache.o
> r
> >>>>>>>>             
> >>>> g
> >>>>     
> >>>>>>> ---------------------------------------------------------------------
> >>>>>>> Terms of use : http://incubator.apache.org/harmony/mailing.html
> >>>>>>> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> >>>>>>> For additional commands, e-mail: harmony-dev-help@incubator.apache.or
> g
> >>>>>>>
> >>>>>>>
> >>>>>>>           
> >>>>>> --
> >>>>>> Alexey A. Petrenko
> >>>>>> Intel Middleware Products Division
> >>>>>>
> >>>>>> ---------------------------------------------------------------------
> >>>>>> Terms of use : http://incubator.apache.org/harmony/mailing.html
> >>>>>> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> >>>>>> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> >>>>>>
> >>>>>>         
> >>>>> ---------------------------------------------------------------------
> >>>>> Terms of use : http://incubator.apache.org/harmony/mailing.html
> >>>>> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> >>>>> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> >>>>>
> >>>>>
> >>>>>       
> >>>> -- 
> >>>> Alexey A. Petrenko
> >>>> Intel Middleware Products Division
> >>>>
> >>>> ---------------------------------------------------------------------
> >>>> Terms of use : http://incubator.apache.org/harmony/mailing.html
> >>>> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> >>>> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> >>>>
> >>>>     
> >>>
> >>>
> >>> ---------------------------------------------------------------------
> >>> Terms of use : http://incubator.apache.org/harmony/mailing.html
> >>> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> >>> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> >>>
> >>>
> >>>   
> >> -- 
> >> Oliver Deakin
> >> IBM United Kingdom Limited
> >>
> >>
> >> ---------------------------------------------------------------------
> >> Terms of use : http://incubator.apache.org/harmony/mailing.html
> >> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> >> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> >>
> > 
> > 
> > 
> > ---------------------------------------------------------------------
> > Terms of use : http://incubator.apache.org/harmony/mailing.html
> > To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> > For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> > 
> 
> ---------------------------------------------------------------------
> Terms of use : http://incubator.apache.org/harmony/mailing.html
> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> 



---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
For additional commands, e-mail: harmony-dev-help@incubator.apache.org


Mime
View raw message