You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ant.apache.org by Stefan Bodewig <bo...@apache.org> on 2010/11/09 13:39:46 UTC
Re: svn commit: r1032922 - /ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/BindTargets.java
On 2010-11-09, <hi...@apache.org> wrote:
> Add a task to bind a target to an extension point.
Might be controversial. What is the use-case?
> public void setTargets(String target) {
> String[] inputs = target.split(",");
Wouldn't a nested element like <ant>'s nested <target> be the better
choice?
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: svn commit: r1032922 - /ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/BindTargets.java
Posted by Stefan Bodewig <bo...@apache.org>.
On 2010-11-09, Stefan Bodewig wrote:
> I've peeked into the thread that followed but not read everything, yet.
Well, I should have, obviously, since everything I added has already
been said by DD and the target-alternative I suggested was already
dismissed.
I see that if the task really is restricted to top level tasks there is
no possibility of changing an already "executing" graph.
I'm +0 on the task by now.
And no, Nicolas, I don't think you have done anything wrong with the way
you handled the commit and you are more than welcome to commit to Ant's
core. Please keep doing it.
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: svn commit: r1032922 - /ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/BindTargets.java
Posted by Stefan Bodewig <bo...@apache.org>.
On 2010-11-09, Nicolas Lalevée wrote:
> Le 9 nov. 2010 à 13:39, Stefan Bodewig a écrit :
>> On 2010-11-09, <hi...@apache.org> wrote:
>>> Add a task to bind a target to an extension point.
>> Might be controversial.
I've peeked into the thread that followed but not read everything, yet.
In no way did I want to imply you should have asked before you committed
the task. I used "controversial" to raise a flag, that's all.
And I may have been too quick with my response to the commit so you
didn't stand a chance of explaining the task.
Historically we have a task by Nikola Ken Barrozzi lurking in bugzilla,
<depend> 12-thousand-and-something (No, I don't know all bug numbers,
I'm cheating, I looked it up earlier when I wrote my first response).
That was turned down and not committed because it would rewrite the
dependency graph at runtime. Your task seems to do the same.
>> What is the use-case?
> The use case is that I have some common shared build files.
> One is a build file which is taking care of publishing artifacts into
> a repository, so have an extension point "ready-to-be-published". I
> also have some other build file which is building a jar, and another
> which is building the source jar, and yet another one which is about
> building the javadoc.
> And now I have two different projects.
> In the first one I want to build a jar, its source and its javadoc,
> and publish all of them. So I my build.xml I import my common-jar.xml,
> common-src-jar.xml common-javadoc-jar.xml and common-publish.xml, and
> "bind" the targets "jar", "src-jar" and "javadoc-jar" to the extension
> "ready-to-be-published". So when calling the target "publish", the 3
> jars will be build and published.
> In the second project I want to be able to build the jar, its source
> and its javadoc, and publish only the jar. So I import my
> common-jar.xml, common-src-jar.xml common-javadoc-jar.xml and
> common-publish.xml, and "bind" only the target "jar" to the extension
> "ready-to-be-published". So when calling the target "publish", only
> the binary jar will be build and published.
To recap whether I understand this. You have a bunch of targets that
you want to add to an extension point. But which target you want to add
depends on some other context, that's why the targets cannot specify the
extension point themselves. Right?
Wouldn't a target that depended on the targets you want to add and adds
itself to the extension point achieve the same effect as your task?
That way the graph would still be static.
So rather than a task
<bindtargets targets="jar,src-jar,javadoc-jar" extensionPoint="publish" />
you'd use
<target name="mypublish" depends="jar,src-jar,javadoc-jar"
extensionOf="publish"/>
>>> public void setTargets(String target) {
>>> String[] inputs = target.split(",");
>> Wouldn't a nested element like <ant>'s nested <target> be the better
>> choice?
> I found it more simple and concise to be just one attribute, like de
> "depends" on the target. Do you think there will be issues with the
> attribute where there won't be with a nested element ?
target names containing commas - they are very uncommon because you
can't use them in depends lists, but still. No, no real issues, just a
matter of style.
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: svn commit: r1032922 - /ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/BindTargets.java
Posted by Dominique Devienne <dd...@gmail.com>.
2010/11/9 Nicolas Lalevée <ni...@hibnet.org>:
>> From the doc you just checked in, I now read:
>>
>> +<p>The bindtargets task may only be used as a top-level task. This means that
>> +it may not be used in a target.</p>
>>
>> So maybe I was wrong. I didn't see the code enforcing that though?
>> What prevents this task from being inside a target?
>
> I have to admit I have blindly trusted the existing code in ImportTask.java. In the execute there is:
> if (getOwningTarget() == null
> || !"".equals(getOwningTarget().getName())) {
> throw new BuildException("import only allowed as a top-level task");
> }
I missed that, thanks. If the doc clearly state that this is just
syntax sugar and show what the equivalent syntax is using <target>,
then I have no further comments ;) --DD
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: svn commit: r1032922 - /ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/BindTargets.java
Posted by Nicolas Lalevée <ni...@hibnet.org>.
Le 9 nov. 2010 à 17:39, Dominique Devienne a écrit :
> On Tue, Nov 9, 2010 at 10:34 AM, Dominique Devienne <dd...@gmail.com> wrote:
>> The reason I'm a little reluctant on <bindtargets> is that it's a task
>> that affects the dependency graph of targets, but bypassing the normal
>> means to do that, via <target>. Since it's a task, it can be run at
>> any time, conditionally or not, inside a target or not, and especially
>> after the dependency graph was computed, when it does/can change the
>> dependency graph. Maybe that's OK, but it just make me a little
>> uncomfortable and I'm not sure we see all the possible ramifications.
>
> From the doc you just checked in, I now read:
>
> +<p>The bindtargets task may only be used as a top-level task. This means that
> +it may not be used in a target.</p>
>
> So maybe I was wrong. I didn't see the code enforcing that though?
> What prevents this task from being inside a target?
I have to admit I have blindly trusted the existing code in ImportTask.java. In the execute there is:
if (getOwningTarget() == null
|| !"".equals(getOwningTarget().getName())) {
throw new BuildException("import only allowed as a top-level task");
}
> PS: Checking the doc with the code might have avoided some confusion ;)
I know I am quite slow, probably doing to much multitasking. Further more when I am dumb enough to forgot to commit them... :p
Nicolas, learning to be an actual ant committer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: svn commit: r1032922 - /ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/BindTargets.java
Posted by Dominique Devienne <dd...@gmail.com>.
On Tue, Nov 9, 2010 at 10:34 AM, Dominique Devienne <dd...@gmail.com> wrote:
> The reason I'm a little reluctant on <bindtargets> is that it's a task
> that affects the dependency graph of targets, but bypassing the normal
> means to do that, via <target>. Since it's a task, it can be run at
> any time, conditionally or not, inside a target or not, and especially
> after the dependency graph was computed, when it does/can change the
> dependency graph. Maybe that's OK, but it just make me a little
> uncomfortable and I'm not sure we see all the possible ramifications.
>From the doc you just checked in, I now read:
+<p>The bindtargets task may only be used as a top-level task. This means that
+it may not be used in a target.</p>
So maybe I was wrong. I didn't see the code enforcing that though?
What prevents this task from being inside a target? --DD
PS: Checking the doc with the code might have avoided some confusion ;)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: svn commit: r1032922 - /ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/BindTargets.java
Posted by Nicolas Lalevée <ni...@hibnet.org>.
Le 9 nov. 2010 à 17:34, Dominique Devienne a écrit :
> 2010/11/9 Nicolas Lalevée <ni...@hibnet.org>:
>> That's what I thought, this proposed task being quite trivial and having no side effect.
>> Obviously for larger patch or behavior change I would come first to the ML, like I did for the project helpers for instance.
>
> Fair enough. A follow up email to the ML is good though, to explain
> rational etc... before the "commit watch patrol" has to ask for it :)
ok, I'll do that next time (if I have enough time before the review comes in :) ).
>
>>>> the use case, or more precisely why the use case you describe can't be
>>>> achieved some other way.
>>
>> It can definitively be handled without that task. With Ant 1.8.1, to bind the targets "jar" and "source" to an extension point "dist" is to create a third target:
>> <target name="bind-to-dist" depends="jar,source" extensionOf="dist" />
>>
>> I find it cleaner to avoid creating yet another target and implement this simple bindtargets task.
>> If there are objection, I'll remove it. Use that work around for classical build files. And put this task in EasyAnt from which I got the idea.
>
> Not being quite up-to-speed on extension-point, I wasn't sure, thanks.
>
> The reason I'm a little reluctant on <bindtargets> is that it's a task
> that affects the dependency graph of targets, but bypassing the normal
> means to do that, via <target>. Since it's a task, it can be run at
> any time, conditionally or not, inside a target or not, and especially
> after the dependency graph was computed, when it does/can change the
> dependency graph. Maybe that's OK, but it just make me a little
> uncomfortable and I'm not sure we see all the possible ramifications.
This target will modify the build graph yes, but no more than the import task.
And just like the import task, this bindtargets task is only executed if it is at the top level.
Nicolas
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: svn commit: r1032922 - /ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/BindTargets.java
Posted by Dominique Devienne <dd...@gmail.com>.
2010/11/9 Nicolas Lalevée <ni...@hibnet.org>:
> That's what I thought, this proposed task being quite trivial and having no side effect.
> Obviously for larger patch or behavior change I would come first to the ML, like I did for the project helpers for instance.
Fair enough. A follow up email to the ML is good though, to explain
rational etc... before the "commit watch patrol" has to ask for it :)
>>> the use case, or more precisely why the use case you describe can't be
>>> achieved some other way.
>
> It can definitively be handled without that task. With Ant 1.8.1, to bind the targets "jar" and "source" to an extension point "dist" is to create a third target:
> <target name="bind-to-dist" depends="jar,source" extensionOf="dist" />
>
> I find it cleaner to avoid creating yet another target and implement this simple bindtargets task.
> If there are objection, I'll remove it. Use that work around for classical build files. And put this task in EasyAnt from which I got the idea.
Not being quite up-to-speed on extension-point, I wasn't sure, thanks.
The reason I'm a little reluctant on <bindtargets> is that it's a task
that affects the dependency graph of targets, but bypassing the normal
means to do that, via <target>. Since it's a task, it can be run at
any time, conditionally or not, inside a target or not, and especially
after the dependency graph was computed, when it does/can change the
dependency graph. Maybe that's OK, but it just make me a little
uncomfortable and I'm not sure we see all the possible ramifications.
It doesn't mean I'm necessarily against it, but if it's only
notational convenience, and the alternative is hardly longer or
"uglier", I'm not sure it's worth it. My $0.02. --DD
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: svn commit: r1032922 - /ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/BindTargets.java
Posted by Nicolas Lalevée <ni...@hibnet.org>.
Le 9 nov. 2010 à 16:27, Matt Benson a écrit :
>
> On Nov 9, 2010, at 9:12 AM, Dominique Devienne wrote:
>
>> 2010/11/9 Nicolas Lalevée <ni...@hibnet.org>:
>>> Note: I'll commit the unit test and doc I have wrote about this task. I don't want to enforce anything, just share the work I have done. It is still up to debate and can still be reverted.
>>
>> Well, process-wise we tend to discuss things out on the ML before
>> committing, or go thru the sandbox.
>
> I wouldn't say that we are always RTC. For changes with potentially large impact, I personally have always gone ahead and opened up discussion beforehand because I didn't want a large changeset to come as a complete surprise. But a particular "expert level" task being added to Ant, I don't really have much problem with.
That's what I thought, this proposed task being quite trivial and having no side effect.
Obviously for larger patch or behavior change I would come first to the ML, like I did for the project helpers for instance.
> I don't 100% see a use for it myself, and am pretty sure that if I did want it, it wouldn't be for simple build composition, but for some kind of parameterized situation. I guess I'm +0 to this task's inclusion.
>
>
>> As Stefan, I still don't quite see
>> the use case, or more precisely why the use case you describe can't be
>> achieved some other way. --DD
It can definitively be handled without that task. With Ant 1.8.1, to bind the targets "jar" and "source" to an extension point "dist" is to create a third target:
<target name="bind-to-dist" depends="jar,source" extensionOf="dist" />
I find it cleaner to avoid creating yet another target and implement this simple bindtargets task.
If there are objection, I'll remove it. Use that work around for classical build files. And put this task in EasyAnt from which I got the idea.
>>
>> PS: There's no enum-like type for onMissingExtensionPoint? Taking it
>> as a string allow passing anithing.
>
> +1 here.
onMissingExtensionPoint being since 1.8.2, it can still be improved. I'll look into making an enum-like class.
Nicolas
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: svn commit: r1032922 - /ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/BindTargets.java
Posted by Matt Benson <gu...@gmail.com>.
On Nov 9, 2010, at 9:12 AM, Dominique Devienne wrote:
> 2010/11/9 Nicolas Lalevée <ni...@hibnet.org>:
>> Note: I'll commit the unit test and doc I have wrote about this task. I don't want to enforce anything, just share the work I have done. It is still up to debate and can still be reverted.
>
> Well, process-wise we tend to discuss things out on the ML before
> committing, or go thru the sandbox.
I wouldn't say that we are always RTC. For changes with potentially large impact, I personally have always gone ahead and opened up discussion beforehand because I didn't want a large changeset to come as a complete surprise. But a particular "expert level" task being added to Ant, I don't really have much problem with. I don't 100% see a use for it myself, and am pretty sure that if I did want it, it wouldn't be for simple build composition, but for some kind of parameterized situation. I guess I'm +0 to this task's inclusion.
> As Stefan, I still don't quite see
> the use case, or more precisely why the use case you describe can't be
> achieved some other way. --DD
>
> PS: There's no enum-like type for onMissingExtensionPoint? Taking it
> as a string allow passing anithing.
+1 here.
-Matt
>
> ---------------------------------------------------------------------
> 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
Re: svn commit: r1032922 - /ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/BindTargets.java
Posted by Matt Benson <gu...@gmail.com>.
On Nov 9, 2010, at 9:12 AM, Dominique Devienne wrote:
> 2010/11/9 Nicolas Lalevée <ni...@hibnet.org>:
>> Note: I'll commit the unit test and doc I have wrote about this task. I don't want to enforce anything, just share the work I have done. It is still up to debate and can still be reverted.
>
> Well, process-wise we tend to discuss things out on the ML before
> committing, or go thru the sandbox.
I wouldn't say that we are always RTC. For changes with potentially large impact, I personally have always gone ahead and opened up discussion beforehand because I didn't want a large changeset to come as a complete surprise. But a particular "expert level" task being added to Ant, I don't really have much problem with. I don't 100% see a use for it myself, and am pretty sure that if I did want it, it wouldn't be for simple build composition, but for some kind of parameterized situation. I guess I'm +0 to this task's inclusion.
> As Stefan, I still don't quite see
> the use case, or more precisely why the use case you describe can't be
> achieved some other way. --DD
>
> PS: There's no enum-like type for onMissingExtensionPoint? Taking it
> as a string allow passing anithing.
+1 here.
-Matt
>
> ---------------------------------------------------------------------
> 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
Re: svn commit: r1032922 - /ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/BindTargets.java
Posted by Dominique Devienne <dd...@gmail.com>.
2010/11/9 Nicolas Lalevée <ni...@hibnet.org>:
> Note: I'll commit the unit test and doc I have wrote about this task. I don't want to enforce anything, just share the work I have done. It is still up to debate and can still be reverted.
Well, process-wise we tend to discuss things out on the ML before
committing, or go thru the sandbox. As Stefan, I still don't quite see
the use case, or more precisely why the use case you describe can't be
achieved some other way. --DD
PS: There's no enum-like type for onMissingExtensionPoint? Taking it
as a string allow passing anithing.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: svn commit: r1032922 - /ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/BindTargets.java
Posted by Nicolas Lalevée <ni...@hibnet.org>.
Le 9 nov. 2010 à 13:39, Stefan Bodewig a écrit :
> On 2010-11-09, <hi...@apache.org> wrote:
>
>> Add a task to bind a target to an extension point.
>
> Might be controversial. What is the use-case?
It is helping when some build files are shared between projects.
The use case is that I have some common shared build files.
One is a build file which is taking care of publishing artifacts into a repository, so have an extension point "ready-to-be-published".
I also have some other build file which is building a jar, and another which is building the source jar, and yet another one which is about building the javadoc.
And now I have two different projects.
In the first one I want to build a jar, its source and its javadoc, and publish all of them. So I my build.xml I import my common-jar.xml, common-src-jar.xml common-javadoc-jar.xml and common-publish.xml, and "bind" the targets "jar", "src-jar" and "javadoc-jar" to the extension "ready-to-be-published". So when calling the target "publish", the 3 jars will be build and published.
In the second project I want to be able to build the jar, its source and its javadoc, and publish only the jar. So I import my common-jar.xml, common-src-jar.xml common-javadoc-jar.xml and common-publish.xml, and "bind" only the target "jar" to the extension "ready-to-be-published". So when calling the target "publish", only the binary jar will be build and published.
In some sense it is about doing composition of build files.
>
>> public void setTargets(String target) {
>> String[] inputs = target.split(",");
>
> Wouldn't a nested element like <ant>'s nested <target> be the better
> choice?
I found it more simple and concise to be just one attribute, like de "depends" on the target. Do you think there will be issues with the attribute where there won't be with a nested element ?
Note: I'll commit the unit test and doc I have wrote about this task. I don't want to enforce anything, just share the work I have done. It is still up to debate and can still be reverted.
Nicolas
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org