You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cocoon.apache.org by Joerg Heinicke <jo...@gmx.de> on 2004/03/12 12:41:37 UTC
Re: cvs commit: cocoon-2.1/tools/src/anttasks XConfToolTask.java
On 11.03.2004 16:11, stephan@apache.org wrote:
> stephan 2004/03/11 07:11:10
>
> Modified: tools/src/anttasks XConfToolTask.java
> Log:
> Retry to apply patches, which depends on each other.
I really wonder why we re-implement the dependency resolving into a task
(how simple it might be) when Ant does it for us. That the patching did
not work until now was probably caused by missing dependencies on target
level, was it not? That means patch-session-fw-block did not depend on
patch-xsp-block, though session-fw block is dependent on xsp block. I
think this should be fixed in the generated blocks-build.xml and so in
the stylesheet generating this file, not in the patch task. This also
probably lowers the reusability of this task.
Joerg
RE: cvs commit: cocoon-2.1/tools/src/anttasks XConfToolTask.java
Posted by Carsten Ziegeler <cz...@s-und-n.de>.
Stephan Michels wrote:
>
> Am Di, den 16.03.2004 schrieb Carsten Ziegeler um 07:58:
> > Hi Stephan, could you please revert your changes? Joerg
> already asked
> > you to do so and I think we should either revert or change
> the current
> > behaviour. It's really annoying to have all this "Dismiss"
> > messages. There are hundreds of them that weren't there before.
>
> The "Dismiss: ..." messages means only that the patch wasn't applied.
> I can easily omit these messages.
>
That would be great!
Carsten
> >
> > > -----Original Message-----
> > > From: Joerg Heinicke [mailto:joerg.heinicke@gmx.de]
> > > Sent: Saturday, March 13, 2004 3:03 PM
> > > To: dev@cocoon.apache.org
> > > Subject: Re: cvs commit: cocoon-2.1/tools/src/anttasks
> > > XConfToolTask.java
> > >
> > > On 12.03.2004 14:29, Stephan Michels wrote:
> > >
> > > > In the orginal form of the blocks-build.xsl, we had
> > > separate targets
> > > > for the patch files. But it was incredible slow. Then I merge
> > > > these targets to one target, and rewrote to the XConf task to a
> > > > MatchingTask, which allow to execute more than one patches.
> > > > But it doesn't preserves the dependencies, then Carsten
> cuts the
> > > > target in to several target again, to solve this problem.
> > > > Now, with latest change it works again.
> > > >
> > > > I tend to agree with you Joerg, separate targets are much
> > > more elegant.
> > > > But in the real world I have real problems, like a build
> > > time von 4min
> > > > 25sec on a 2.4GHz Intel system. Which is, by the way,
> > > > unacceptable, IMHO.
> > > >
> > > > So, should I revert the change to have a more elegant build
> > > file with
> > > > bigger build time?! .... ehrmm ... I think not.
> > >
> > > To be honest, such statements enrage me at least a bit. You talk
> > > about time, but you forget the time to maintain this additional
> > > dependency resolving. Starting with the missing .xweb patches you
> > > have now to go on searching for bugs - things that
> already have been
> > > working. For having a look on this issue I removed ojb,
> database and
> > > hsqldb block from the excluded ones. A simple build (Cocoon was
> > > previously built with only cforms and xsp enabled) - and many
> > > patches of those blocks were not applied. Only a clean
> build made it
> > > working - partly, see above. If I need to do every time a clean
> > > build to get this thing correctly working, I don't see
> how you can
> > > gain time. This might be only a simple bug somewhere,
> maybe only a
> > > typo - but I talk about the principle - which, I know,
> often ends in
> > > obstinacy.
>
> Was a minor bug, should now be solved.
>
> > > IMO, yes, we should revert it. I prefer the elegancy much
> more about
> > > the speed. And to add Carsten's argument:
> > > Additionally it forces us "keeping the dependencies correct".
> > >
> > > Excuse me, if I have forgotten to add 'rant' around it ...
>
> Yes, you have.
>
> But okay, when I will revert my changes to version 1.17 from
> 2003/05/05
>
> Stephan.
>
RE: cvs commit: cocoon-2.1/tools/src/anttasks XConfToolTask.java
Posted by Stephan Michels <st...@apache.org>.
Am Di, den 16.03.2004 schrieb Carsten Ziegeler um 07:58:
> Hi Stephan, could you please revert your changes? Joerg already
> asked you to do so and I think we should either revert or change
> the current behaviour. It's really annoying to have all this "Dismiss"
> messages. There are hundreds of them that weren't there before.
The "Dismiss: ..." messages means only that the patch wasn't applied.
I can easily omit these messages.
>
> > -----Original Message-----
> > From: Joerg Heinicke [mailto:joerg.heinicke@gmx.de]
> > Sent: Saturday, March 13, 2004 3:03 PM
> > To: dev@cocoon.apache.org
> > Subject: Re: cvs commit: cocoon-2.1/tools/src/anttasks
> > XConfToolTask.java
> >
> > On 12.03.2004 14:29, Stephan Michels wrote:
> >
> > > In the orginal form of the blocks-build.xsl, we had
> > separate targets
> > > for the patch files. But it was incredible slow. Then I merge these
> > > targets to one target, and rewrote to the XConf task to a
> > > MatchingTask, which allow to execute more than one patches.
> > > But it doesn't preserves the dependencies, then Carsten cuts the
> > > target in to several target again, to solve this problem.
> > > Now, with latest change it works again.
> > >
> > > I tend to agree with you Joerg, separate targets are much
> > more elegant.
> > > But in the real world I have real problems, like a build
> > time von 4min
> > > 25sec on a 2.4GHz Intel system. Which is, by the way, unacceptable,
> > > IMHO.
> > >
> > > So, should I revert the change to have a more elegant build
> > file with
> > > bigger build time?! .... ehrmm ... I think not.
> >
> > To be honest, such statements enrage me at least a bit. You
> > talk about time, but you forget the time to maintain this
> > additional dependency resolving. Starting with the missing
> > .xweb patches you have now to go on searching for bugs -
> > things that already have been working. For having a look on
> > this issue I removed ojb, database and hsqldb block from the
> > excluded ones. A simple build (Cocoon was previously built
> > with only cforms and xsp enabled) - and many patches of those
> > blocks were not applied. Only a clean build made it working -
> > partly, see above. If I need to do every time a clean build
> > to get this thing correctly working, I don't see how you can
> > gain time. This might be only a simple bug somewhere, maybe
> > only a typo - but I talk about the principle - which, I know,
> > often ends in obstinacy.
Was a minor bug, should now be solved.
> > IMO, yes, we should revert it. I prefer the elegancy much
> > more about the speed. And to add Carsten's argument:
> > Additionally it forces us "keeping the dependencies correct".
> >
> > Excuse me, if I have forgotten to add 'rant' around it ...
Yes, you have.
But okay, when I will revert my changes to version 1.17 from 2003/05/05
Stephan.
RE: cvs commit: cocoon-2.1/tools/src/anttasks XConfToolTask.java
Posted by Carsten Ziegeler <cz...@s-und-n.de>.
Hi Stephan, could you please revert your changes? Joerg already
asked you to do so and I think we should either revert or change
the current behaviour. It's really annoying to have all this "Dismiss"
messages. There are hundreds of them that weren't there before.
Carsten
> -----Original Message-----
> From: Joerg Heinicke [mailto:joerg.heinicke@gmx.de]
> Sent: Saturday, March 13, 2004 3:03 PM
> To: dev@cocoon.apache.org
> Subject: Re: cvs commit: cocoon-2.1/tools/src/anttasks
> XConfToolTask.java
>
> On 12.03.2004 14:29, Stephan Michels wrote:
>
> > In the orginal form of the blocks-build.xsl, we had
> separate targets
> > for the patch files. But it was incredible slow. Then I merge these
> > targets to one target, and rewrote to the XConf task to a
> > MatchingTask, which allow to execute more than one patches.
> > But it doesn't preserves the dependencies, then Carsten cuts the
> > target in to several target again, to solve this problem.
> > Now, with latest change it works again.
> >
> > I tend to agree with you Joerg, separate targets are much
> more elegant.
> > But in the real world I have real problems, like a build
> time von 4min
> > 25sec on a 2.4GHz Intel system. Which is, by the way, unacceptable,
> > IMHO.
> >
> > So, should I revert the change to have a more elegant build
> file with
> > bigger build time?! .... ehrmm ... I think not.
>
> To be honest, such statements enrage me at least a bit. You
> talk about time, but you forget the time to maintain this
> additional dependency resolving. Starting with the missing
> .xweb patches you have now to go on searching for bugs -
> things that already have been working. For having a look on
> this issue I removed ojb, database and hsqldb block from the
> excluded ones. A simple build (Cocoon was previously built
> with only cforms and xsp enabled) - and many patches of those
> blocks were not applied. Only a clean build made it working -
> partly, see above. If I need to do every time a clean build
> to get this thing correctly working, I don't see how you can
> gain time. This might be only a simple bug somewhere, maybe
> only a typo - but I talk about the principle - which, I know,
> often ends in obstinacy.
>
> IMO, yes, we should revert it. I prefer the elegancy much
> more about the speed. And to add Carsten's argument:
> Additionally it forces us "keeping the dependencies correct".
>
> Excuse me, if I have forgotten to add 'rant' around it ...
>
> Joerg
>
>
Re: cvs commit: cocoon-2.1/tools/src/anttasks XConfToolTask.java
Posted by Joerg Heinicke <jo...@gmx.de>.
On 12.03.2004 14:29, Stephan Michels wrote:
> In the orginal form of the blocks-build.xsl, we had separate targets for
> the patch files. But it was incredible slow. Then I merge these targets
> to one target, and rewrote to the XConf task to a MatchingTask, which
> allow to execute more than one patches.
> But it doesn't preserves the dependencies, then Carsten cuts the target
> in to several target again, to solve this problem.
> Now, with latest change it works again.
>
> I tend to agree with you Joerg, separate targets are much more elegant.
> But in the real world I have real problems, like a build time von 4min
> 25sec on a 2.4GHz Intel system. Which is, by the way, unacceptable,
> IMHO.
>
> So, should I revert the change to have a more elegant build file with
> bigger build time?! .... ehrmm ... I think not.
To be honest, such statements enrage me at least a bit. You talk about
time, but you forget the time to maintain this additional dependency
resolving. Starting with the missing .xweb patches you have now to go on
searching for bugs - things that already have been working. For having a
look on this issue I removed ojb, database and hsqldb block from the
excluded ones. A simple build (Cocoon was previously built with only
cforms and xsp enabled) - and many patches of those blocks were not
applied. Only a clean build made it working - partly, see above. If I
need to do every time a clean build to get this thing correctly working,
I don't see how you can gain time. This might be only a simple bug
somewhere, maybe only a typo - but I talk about the principle - which, I
know, often ends in obstinacy.
IMO, yes, we should revert it. I prefer the elegancy much more about the
speed. And to add Carsten's argument: Additionally it forces us "keeping
the dependencies correct".
Excuse me, if I have forgotten to add 'rant' around it ...
Joerg
Re: cvs commit: cocoon-2.1/tools/src/anttasks XConfToolTask.java
Posted by Stephan Michels <st...@apache.org>.
Am Fr, den 12.03.2004 schrieb Joerg Heinicke um 13:30:
> On 12.03.2004 13:01, Carsten Ziegeler wrote:
>
> >>I thought that previously all xpatches for all blocks were
> >>executed in one go instead of separately and respecting
> >>dependency order.
> >
> > No, one patch after the other was applied previously. The order
> > of the dependencies was used to define the order of the patches
> > to be applied.
> > So, if the dependencies were correctly set, no problem could occur.
>
> Where do you take this from? From what I see at [1] there was exactly
> one patch-conf target, for the dependencies we would need one target for
> every cocoon block. Or do I miss something?
In the orginal form of the blocks-build.xsl, we had separate targets for
the patch files. But it was incredible slow. Then I merge these targets
to one target, and rewrote to the XConf task to a MatchingTask, which
allow to execute more than one patches.
But it doesn't preserves the dependencies, then Carsten cuts the target
in to several target again, to solve this problem.
Now, with latest change it works again.
I tend to agree with you Joerg, separate targets are much more elegant.
But in the real world I have real problems, like a build time von 4min
25sec on a 2.4GHz Intel system. Which is, by the way, unacceptable,
IMHO.
So, should I revert the change to have a more elegant build file with
bigger build time?! .... ehrmm ... I think not.
Stephan.
RE: cvs commit: cocoon-2.1/tools/src/anttasks XConfToolTask.java
Posted by Carsten Ziegeler <cz...@s-und-n.de>.
Unico Hommes wrote:
> Now should the changes to XConfToolTask be rolled back? I
> think so, unless it has other advantages.
>
Yes, it helps keeping the dependencies correct.
Carsten
Re: cvs commit: cocoon-2.1/tools/src/anttasks XConfToolTask.java
Posted by Unico Hommes <un...@hippo.nl>.
Carsten Ziegeler wrote:
>Joerg Heinicke wrote:
>
>
>>On 12.03.2004 13:01, Carsten Ziegeler wrote:
>>
>>
>>
>>>>I thought that previously all xpatches for all blocks were
>>>>
>>>>
>>executed in
>>
>>
>>>>one go instead of separately and respecting dependency order.
>>>>
>>>>
>>>No, one patch after the other was applied previously. The
>>>
>>>
>>order of the
>>
>>
>>>dependencies was used to define the order of the patches to be
>>>applied.
>>>So, if the dependencies were correctly set, no problem could occur.
>>>
>>>
>>Where do you take this from?
>>
>>
>Because I did it :)
>
>>From what I see at [1] there was
>
>
>>exactly one patch-conf target, for the dependencies we would
>>need one target for every cocoon block. Or do I miss something?
>>
>>
>>
>Actually, I don't know anymore...I only know before I change that,
>the dependencies weren't preserved, with the changes it worked
>very well.
>But that's the past anyway :)
>
>
>
Ok I understand now. There is only one global patch-conf target and it
preserves dependencies. So my previous assertion that the problem with
session-fw dependency needed a change in the build system was incorrect.
It should have been just a matter of declaring the correct dependency.
Check.
Now should the changes to XConfToolTask be rolled back? I think so,
unless it has other advantages.
--
Unico
RE: cvs commit: cocoon-2.1/tools/src/anttasks XConfToolTask.java
Posted by Carsten Ziegeler <cz...@s-und-n.de>.
Joerg Heinicke wrote:
>
> On 12.03.2004 13:01, Carsten Ziegeler wrote:
>
> >>I thought that previously all xpatches for all blocks were
> executed in
> >>one go instead of separately and respecting dependency order.
> >
> > No, one patch after the other was applied previously. The
> order of the
> > dependencies was used to define the order of the patches to be
> > applied.
> > So, if the dependencies were correctly set, no problem could occur.
>
> Where do you take this from?
Because I did it :)
>From what I see at [1] there was
> exactly one patch-conf target, for the dependencies we would
> need one target for every cocoon block. Or do I miss something?
>
Actually, I don't know anymore...I only know before I change that,
the dependencies weren't preserved, with the changes it worked
very well.
But that's the past anyway :)
Carsten
Re: cvs commit: cocoon-2.1/tools/src/anttasks XConfToolTask.java
Posted by Joerg Heinicke <jo...@gmx.de>.
On 12.03.2004 13:01, Carsten Ziegeler wrote:
>>I thought that previously all xpatches for all blocks were
>>executed in one go instead of separately and respecting
>>dependency order.
>
> No, one patch after the other was applied previously. The order
> of the dependencies was used to define the order of the patches
> to be applied.
> So, if the dependencies were correctly set, no problem could occur.
Where do you take this from? From what I see at [1] there was exactly
one patch-conf target, for the dependencies we would need one target for
every cocoon block. Or do I miss something?
Joerg
[1]
http://cvs.apache.org/viewcvs.cgi/cocoon-2.1/tools/src/blocks-build.xsl?annotate=1.48#231
RE: cvs commit: cocoon-2.1/tools/src/anttasks XConfToolTask.java
Posted by Carsten Ziegeler <cz...@s-und-n.de>.
Unico Hommes wrote:
> I thought that previously all xpatches for all blocks were
> executed in one go instead of separately and respecting
> dependency order.
No, one patch after the other was applied previously. The order
of the dependencies was used to define the order of the patches
to be applied.
So, if the dependencies were correctly set, no problem could occur.
Carsten
> And that this was the root of the problem.
> But I am not sure. I also would prefer using the dependency
> information to dynamically arrange patch order instead.
>
Re: cvs commit: cocoon-2.1/tools/src/anttasks XConfToolTask.java
Posted by Unico Hommes <un...@hippo.nl>.
Joerg Heinicke wrote:
> On 11.03.2004 16:11, stephan@apache.org wrote:
>
>> stephan 2004/03/11 07:11:10
>>
>> Modified: tools/src/anttasks XConfToolTask.java
>> Log:
>> Retry to apply patches, which depends on each other.
>
>
> I really wonder why we re-implement the dependency resolving into a
> task (how simple it might be) when Ant does it for us. That the
> patching did not work until now was probably caused by missing
> dependencies on target level, was it not? That means
> patch-session-fw-block did not depend on patch-xsp-block, though
> session-fw block is dependent on xsp block. I think this should be
> fixed in the generated blocks-build.xml and so in the stylesheet
> generating this file, not in the patch task. This also probably lowers
> the reusability of this task.
>
I thought that previously all xpatches for all blocks were executed in
one go instead of separately and respecting dependency order. And that
this was the root of the problem. But I am not sure. I also would prefer
using the dependency information to dynamically arrange patch order instead.
--
Unico