ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From peter reilly <peter.rei...@corvil.com>
Subject Re: Vote: <local> for 1.6
Date Thu, 23 Oct 2003 11:43:05 GMT
On Thursday 23 October 2003 11:09, Jose Alberto Fernandez wrote:
> Ok, I think I will try to go back to the drawing board and try
> to provide my simple definition of <local> as I originally
> proposed. I guess I did not look close enough your implementation.
>
> From your example bellow I see that <local> does not define a proper
> nesting context (which I think is wrong). In my original proposal
> <local> extended <sequential> and as such the life of the <local>
> was restricted to that nesting. For what I see in your example
> <local> is not defined as a nesting task and that concerns me.

The nesting is for the enclosing target, sequential or target.
When the nesting task ends, the local's are removed. To support
multiple locals in the same scope, a solution would look much
like the current one. One example would be the way lisp does things.

  (let ((here (point))
        (ignore-depth 0)
        (tag-info context))
    bla.. bla..
  )
Converted to ant-xml, this would look something like:
<let>
   <locals>
      <local name="here" value="point"/>
      <local name="ignore-depth value="0"/>
      <local name="tag-info" value="context"/>
    </locals>
   <echo>here is ${here}</echo>
</let>
Dropping the locals (as "local" should be sufficient information)
<let>
  <local name="here" value="point"/>
  <local name="ignore-depth value="0"/>
  <local name="tag-info" value="context"/>
  <echo>here is ${here}</echo>
</let>

But this is not much different from: 
<sequential>
  <local name="here" value="point"/>
  <local name="ignore-depth value="0"/>
  <local name="tag-info" value="context"/>
  <echo>here is ${here}</echo>
</sequential>

or:
<target name="sample">
  <local name="here" value="point"/>
  <local name="ignore-depth value="0"/>
  <local name="tag-info" value="context"/>
  <echo>here is ${here}</echo>
</target>

>
> As per the <attribute> of <macrodef>, I tend to agree with you
> that probably it is not a good idea to treat it as a <local>
> since its expansion should be just on the expansion of the macro
> itself. But I think the issue here is in <macrodef> and not in the
> definition of <local>.

I think that the same argument applies to [sub]ant[call] - a
solution should deal with them and macro instances in the same way.

>
> Finally, I would argue that the burden of using ThreadLocals or
> watever should be on <parallel> and not is some other task or
> the CORE as a whole. If you want to parallelize tasks that
> interfere with one another, it should be upto to parallel
> to allow for independent computations. This should be
> a completely separate issue from that of <local> it should
> be an enhancement request on <parallel>.

(This is also in reply to Conor's e-mail yesterday about ).

The code needs to handle a number of different situations. But
one thing needs to be kept in mind. As the local properties are
seen as normal properties by tasks, they use project[Helper] to access
the property. This project pointer is the same for all the threads in
a parallel - so if the threads need to see different properties for the
same name the only way projectHelper knowns this is by the thread id.
Java provides ThreadLocal for this situation.

An example of this is:

<macrodef name="example">
  <attribute name="val"/>
  <sequential>
     <echo>val is ${val}</echo>
  </sequential>
</macrodef>

<parallel>
   <example val="1"/>
   <example val="2"/>
   <example val="3"/>
</parallel>

So the echo task whould need to see "1", "2" and "3" in the different threads
at the same time using getProject().getProperty("val");

The PropertyHelper code does this by using InheritableThreadLocal and
makes a copy for each thread.

As this is done, why does parallel have code to also copy ?

This is done to support the following:
<local name="x"/>
<parallel>
   <sequential>
       <property name="x" value="1"/>
       <echo>x is ${x}</echo>
   </sequential>
   <sequential>
       <property name="x" value="2"/>
       <echo>x is ${x}</echo>
   </sequential>
   <sequential>
       <property name="x" value="3"/>
       <echo>x is ${x}</echo>
   </sequential>
</parallel>

One should see x is 1 and x is 2 etc (in some order).

parallel makes use of a thread pool, and so unless it copies the locals, some
of the sequentials would have the x set in their context.

So if this is the case, why does PropertyHelper copy the local variables?

This is done because the code is paranoid. A third party implemention
of something like <parallel> may not copy the local properties.

Peter
>
> So lets talk this over some more,
>
> Jose Alberto
>
> > -----Original Message-----
> > From: peter reilly [mailto:peter.reilly@corvil.com]
> > Sent: 22 October 2003 18:36
> > To: Ant Developers List
> > Subject: Re: Vote: <local> for 1.6
> >
> > On Wednesday 22 October 2003 15:04, Stefan Bodewig wrote:
> > > On 22 Oct 2003, Stefan Bodewig <bodewig@apache.org> wrote:
> > > > I haven't made up my mind on the feature itself
> > >
> > > OK, re-reading the description first, I don't like the
> > >
> > > ,----
> > >
> > > | The value part of <local> is optional, and  the local
> >
> > property may
> >
> > > | be set by a subsequent <property>, <property> will only set it
if
> > > | the value is not set.
> > >
> > > `----
> > >
> > > part.  This means that whenever I find a <property> task
> >
> > I'll have to
> >
> > > search all possible scopes for a <local> element and can't
> >
> > rely on it
> >
> > > being global.
> > >
> > > What is the benefit of making <property> adhere to the
> >
> > scoping set up
> >
> > > by <local>?
> >
> > The point is that all tasks including <property> see the
> > local properties as normal properties.
> >
> > It means that the macro attributes are now seen as normal
> > properties by the tasks that are contained within the
> > sequential nested element.
> >
> > The <local> makes a property of the same kind as the macro attribute.
> >
> > A common use case for this is when converting an antcall to a
> > macro: I had a target that was used as an antcall target, it
> > used a property "suite.pattern" to contain a regex what was
> > used a number of times in the target and as an ant call was
> > done to invoke the target, this was not seen outside the
> > target. On conversion to a macrodef, the property
> > "suite.pattern" is now a global property and the macrodef may
> > fail unexpectly.
> >
> > With local the macro now looks like this:
> >   <!--
> >        macro to generate test suites registing
> >        @param gen.dir  the dir to place the register_suites.h
> > and .cpp files
> >        @param unit.dir the dir that the unit_*.cpp files are located
> >        -->
> >   <macrodef name="gen-register-suites">
> >     <attribute name="gen.dir"/>
> >     <attribute name="unit.dir"/>
> >     <sequential>
> >       <local name="suite.pattern" value="^ *SUITE\(.*,\s*(.*)\s*\)"/>
> >       <mkdir dir="${gen.dir}"/>
> >        ...
> >     </sequential>
> >  </macrodef>
> >
> > Another common use case is use of a local to pass information
> > from one task to another without messing up global properties
> > or similar properties used in other targets (say in a
> > complicated build with a number of imports and lots of targets).
> >
> >    <local name="filepath"/>
> >    <pathconvert property="filepath" targetos="unix"
> >                 refid="files.path"/>
> >    <echo>the files in "files.path" are ${filepath}</echo>
> >
> > However I can see the problem of looking at a script and not
> > knowing if the property is local or global.
> >
> > The last patch allowed [sub]ant[call] to inherit the local properties.
> >
> > I propose to change the local so that this inheriatance is
> > removed and also that macro instances do not inherit the
> > local properites - i.e. the local properties are staticlly
> > scoped in the build script.
> >
> > So currently the following is the case:
> >     <property name="p" value="global"/>
> >
> >     <macrodef name="inner">
> >       <sequential>
> >         <echo>p is ${p}</echo>
> >       </sequential>
> >     </macrodef>
> >
> >     <macrodef name="outer">
> >       <attribute name="p"/>
> >        <sequential>
> >           <inner/>
> >        </sequential>
> >     </macrodef>
> >
> >     <outer p="from macro"/>
> >
> > Will generate "p is from macro" which is probally not
> > expected as inner was not explicilty passed the property by
> > outer and inner may be in another file or in an antlib.xml.
> >
> > Peter
> >
> > > I don't have any strong objections against the rest or the
> > > implementation.
> > >
> > > So +0.5 (which can be turned into a +0.9 by explaining why
> >
> > <property>
> >
> > > should care about scopes 8-).
> > >
> > > Stefan
> >
> > ---------------------------------------------------------------------
> >
> > > To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> > > For additional commands, e-mail: dev-help@ant.apache.org
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> > For additional commands, e-mail: dev-help@ant.apache.org
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> For additional commands, e-mail: dev-help@ant.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Mime
View raw message