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:05:14 GMT
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 :)

geir


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

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