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 09:07:40 GMT

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> wrote:
> 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@gmail
> .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>
wrot
> >>>>>           
> >> 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 to
> >>>>>> ignore though.
> >>>>>>             
> >>>>> Yes.  I considered that and had the same concern.  Even if I wrote
the
> >>>>> code to print the warning, I think I'd ignore it since it would
scroll
> >>>>> 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.or
> >>>>>>             
> >> 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.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
> >>>
> >>>
> >>>       
> >> -- 
> >> 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


Mime
View raw message