harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Geir Magnusson Jr." <g...@pobox.com>
Subject Re: [classlib] Trying to catch patternset errors earlier
Date Thu, 05 Oct 2006 14:35:52 GMT


Mark Hindess wrote:
> 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?
> 

I've grep-ed them looking for stuff.

I wonder if it would be easy to write out a "map" file.

eh, never mind.  We'll see if anyone cares...

geir


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

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