DO NOT REPLY [Bug 28444] - Import: Target Handling Bug

classic Classic list List threaded Threaded
42 messages Options
123
Reply | Threaded
Open this post in threaded view
|

DO NOT REPLY [Bug 28444] - Import: Target Handling Bug

Bugzilla from bugzilla@apache.org
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG?
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=28444>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND?
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=28444





------- Additional Comments From [hidden email]  2005-05-12 16:24 -------
I never liked the target renaming stuff - it seems a bit strange.
Be that as it may, the current behaviour is a bit silly - i.e. inconsistent.
The target gets renamed if there another target of the same name, but
not otherwise - how can one write a proper reusable import file using the
rename feature in this case?

The fix will have a small overhead - the target object needs to be
cloned and given a a new name, whereas the current code just renames the
target object.

--
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: DO NOT REPLY [Bug 28444] - Import: Target Handling Bug

Stefan Bodewig
On Thu, 12 May 2005, <[hidden email]> wrote:

> Additional Comments From [hidden email]

> Be that as it may, the current behaviour is a bit silly -
> i.e. inconsistent.

big +1

But we should discuss it here instead of in bugzilla - nobody's going
to follow it there.  I know that I don't.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [Bug 28444] - Import: Target Handling Bug

peterreilly
Stefan Bodewig wrote:

>>Be that as it may, the current behaviour is a bit silly -
>>i.e. inconsistent.
>>    
>>
>
>big +1
>
>But we should discuss it here instead of in bugzilla - nobody's going
>to follow it there.  I know that I don't.
>  
>
No problem.

The bug is : http://issues.apache.org/bugzilla/show_bug.cgi?id=28444
The example given is:

<project name="A">
   <target name="x"/>
</project>

<project name="B">
   <import file="A.xml"/>
   <target name="x" depends="A.x"/>
</project>

<project name="C">
   <import file="A.xml"/>
   <import file="B.xml"/>
</project>

Succeeds:
   ant -f A.xml x
   ant -f B.xml x

Fails:
   ant -f C.xml x

BUILD FAILED


Target `A.x' does not exist in this project. It is used from target `B.x'.


My comment is:

I never liked the target renaming stuff - it seems a bit strange.
Be that as it may, the current behaviour is a bit silly - i.e. inconsistent.
The target gets renamed if there another target of the same name, but
not otherwise - how can one write a proper reusable import file using the
rename feature in this case?

The fix will have a small overhead - the target object needs to be
cloned and given a a new name, whereas the current code just renames the
target object.

The changes to ant for this is:
Index: src/main/org/apache/tools/ant/Target.java
===================================================================
RCS file: /home/cvs/ant/src/main/org/apache/tools/ant/Target.java,v
retrieving revision 1.58
diff -u -3 -p -r1.58 Target.java
--- src/main/org/apache/tools/ant/Target.java   10 Mar 2005 12:50:57 -0000      1.58
+++ src/main/org/apache/tools/ant/Target.java   12 May 2005 14:41:25 -0000
@@ -52,12 +52,28 @@ public class Target implements TaskConta
     /** Description of this target, if any. */
     private String description = null;

-    /** Sole constructor. */
+    /** Default constructor. */
     public Target() {
         //empty
     }

     /**
+     * Cloning constructor.
+     * @param other the Target to clone.
+     */
+    public Target(Target other) {
+        this.name = other.name;
+        this.ifCondition = other.ifCondition;
+        this.unlessCondition = other.unlessCondition;
+        this.dependencies = other.dependencies;
+        this.location = other.location;
+        this.project = other.project;
+        this.description = other.description;
+        // The children are added to after this cloning
+        this.children = other.children;
+    }
+
+    /**
      * Sets the project this target belongs to.
      *
      * @param project The project this target belongs to.
Index: src/main/org/apache/tools/ant/helper/ProjectHelper2.java
===================================================================
RCS file: /home/cvs/ant/src/main/org/apache/tools/ant/helper/ProjectHelper2.java,v
retrieving revision 1.54
diff -u -3 -p -r1.54 ProjectHelper2.java
--- src/main/org/apache/tools/ant/helper/ProjectHelper2.java    26 Apr 2005 11:55:18 -0000    1.54
+++ src/main/org/apache/tools/ant/helper/ProjectHelper2.java    12 May 2005 14:41:26 -0000
@@ -815,38 +815,39 @@ public class ProjectHelper2 extends Proj
                     + "a name attribute", context.getLocator());
             }

-            Hashtable currentTargets = project.getTargets();
+            // Check if this target is in the current build file
+            if (context.getCurrentTargets().get(name) != null) {
+                throw new BuildException(
+                    "Duplicate target '" + name + "'", target.getLocation());
+            }

-            // If the name has already been defined ( import for example )
-            if (currentTargets.containsKey(name)) {
-                if (context.getCurrentTargets().get(name) != null) {
-                    throw new BuildException(
-                        "Duplicate target '" + name + "'", target.getLocation());
-                }
-                // Alter the name.
-                if (context.getCurrentProjectName() != null) {
-                    String newName = context.getCurrentProjectName()
-                        + "." + name;
-                    project.log("Already defined in main or a previous import, "
-                        + "define " + name + " as " + newName,
-                                Project.MSG_VERBOSE);
-                    name = newName;
-                } else {
-                    project.log("Already defined in main or a previous import, "
-                        + "ignore " + name, Project.MSG_VERBOSE);
-                    name = null;
-                }
+            if (depends.length() > 0) {
+                target.setDepends(depends);
             }

-            if (name != null) {
+            Hashtable projectTargets = project.getTargets();
+            boolean   usedTarget = false;
+            // If the name has not already been defined define it
+            if (projectTargets.containsKey(name)) {
+                project.log("Already defined in main or a previous import, "
+                            + "ignore " + name, Project.MSG_VERBOSE);
+            } else {
                 target.setName(name);
                 context.getCurrentTargets().put(name, target);
                 project.addOrReplaceTarget(name, target);
+                usedTarget = true;
             }

-            // take care of dependencies
-            if (depends.length() > 0) {
-                target.setDepends(depends);
+            if (context.isIgnoringProjectTag() && context.getCurrentProjectName() != null
+                && context.getCurrentProjectName().length() != 0) {
+                // In an impored file (and not completely
+                // ignoring the project tag)
+                String newName = context.getCurrentProjectName()
+                    + "." + name;
+                Target newTarget = usedTarget ? new Target(target) : target;
+                newTarget.setName(newName);
+                context.getCurrentTargets().put(newName, newTarget);
+                project.addOrReplaceTarget(newName, newTarget);
             }
         }

Cheers,
Peter



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [Bug 28444] - Import: Target Handling Bug

Matt Benson
Here's my comment from the bug since Stefan wants to
be on-list:  :)

<quote>
I also support fixing this.  However, instead of
cloning the entire target,
couldn't we:

a) unconditionally rename the imported target
b) if the target has NOT been overridden, create a new
target that depends on
the imported target

?
-Matt
</quote>

The only issue with this is logging.  I guess it
really doesn't matter; the cloning approach shouldn't
be much heavier since it's as shallow as it is...

:)

--- Peter Reilly <[hidden email]> wrote:

> Stefan Bodewig wrote:
>
> >>Be that as it may, the current behaviour is a bit
> silly -
> >>i.e. inconsistent.
> >>    
> >>
> >
> >big +1
> >
> >But we should discuss it here instead of in
> bugzilla - nobody's going
> >to follow it there.  I know that I don't.
> >  
> >
> No problem.
>
> The bug is :
>
http://issues.apache.org/bugzilla/show_bug.cgi?id=28444

> The example given is:
>
> <project name="A">
>    <target name="x"/>
> </project>
>
> <project name="B">
>    <import file="A.xml"/>
>    <target name="x" depends="A.x"/>
> </project>
>
> <project name="C">
>    <import file="A.xml"/>
>    <import file="B.xml"/>
> </project>
>
> Succeeds:
>    ant -f A.xml x
>    ant -f B.xml x
>
> Fails:
>    ant -f C.xml x
>
> BUILD FAILED
>
>
> Target `A.x' does not exist in this project. It is
> used from target `B.x'.
>
>
> My comment is:
>
> I never liked the target renaming stuff - it seems a
> bit strange.
> Be that as it may, the current behaviour is a bit
> silly - i.e. inconsistent.
> The target gets renamed if there another target of
> the same name, but
> not otherwise - how can one write a proper reusable
> import file using the
> rename feature in this case?
>
> The fix will have a small overhead - the target
> object needs to be
> cloned and given a a new name, whereas the current
> code just renames the
> target object.
>
> The changes to ant for this is:
> Index: src/main/org/apache/tools/ant/Target.java
>
===================================================================
> RCS file:
>
/home/cvs/ant/src/main/org/apache/tools/ant/Target.java,v

> retrieving revision 1.58
> diff -u -3 -p -r1.58 Target.java
> --- src/main/org/apache/tools/ant/Target.java   10
> Mar 2005 12:50:57 -0000      1.58
> +++ src/main/org/apache/tools/ant/Target.java   12
> May 2005 14:41:25 -0000
> @@ -52,12 +52,28 @@ public class Target implements
> TaskConta
>      /** Description of this target, if any. */
>      private String description = null;
>
> -    /** Sole constructor. */
> +    /** Default constructor. */
>      public Target() {
>          //empty
>      }
>
>      /**
> +     * Cloning constructor.
> +     * @param other the Target to clone.
> +     */
> +    public Target(Target other) {
> +        this.name = other.name;
> +        this.ifCondition = other.ifCondition;
> +        this.unlessCondition =
> other.unlessCondition;
> +        this.dependencies = other.dependencies;
> +        this.location = other.location;
> +        this.project = other.project;
> +        this.description = other.description;
> +        // The children are added to after this
> cloning
> +        this.children = other.children;
> +    }
> +
> +    /**
>       * Sets the project this target belongs to.
>       *
>       * @param project The project this target
> belongs to.
> Index:
>
src/main/org/apache/tools/ant/helper/ProjectHelper2.java
>
===================================================================
> RCS file:
>
/home/cvs/ant/src/main/org/apache/tools/ant/helper/ProjectHelper2.java,v
> retrieving revision 1.54
> diff -u -3 -p -r1.54 ProjectHelper2.java
> ---
>
src/main/org/apache/tools/ant/helper/ProjectHelper2.java
>    26 Apr 2005 11:55:18 -0000    1.54
> +++
>
src/main/org/apache/tools/ant/helper/ProjectHelper2.java

>    12 May 2005 14:41:26 -0000
> @@ -815,38 +815,39 @@ public class ProjectHelper2
> extends Proj
>                      + "a name attribute",
> context.getLocator());
>              }
>
> -            Hashtable currentTargets =
> project.getTargets();
> +            // Check if this target is in the
> current build file
> +            if
> (context.getCurrentTargets().get(name) != null) {
> +                throw new BuildException(
> +                    "Duplicate target '" + name +
> "'", target.getLocation());
> +            }
>
> -            // If the name has already been defined
> ( import for example )
> -            if (currentTargets.containsKey(name)) {
> -                if
> (context.getCurrentTargets().get(name) != null) {
> -                    throw new BuildException(
> -                        "Duplicate target '" + name
> + "'", target.getLocation());
> -                }
> -                // Alter the name.
> -                if (context.getCurrentProjectName()
> != null) {
> -                    String newName =
> context.getCurrentProjectName()
> -                        + "." + name;
> -                    project.log("Already defined in
> main or a previous import, "
> -                        + "define " + name + " as "
> + newName,
> -                              
> Project.MSG_VERBOSE);
> -                    name = newName;
> -                } else {
> -                    project.log("Already defined in
> main or a previous import, "
> -                        + "ignore " + name,
> Project.MSG_VERBOSE);
> -                    name = null;
> -                }
> +            if (depends.length() > 0) {
> +                target.setDepends(depends);
>              }
>
> -            if (name != null) {
> +            Hashtable projectTargets =
> project.getTargets();
> +            boolean   usedTarget = false;
> +            // If the name has not already been
> defined define it
> +            if (projectTargets.containsKey(name)) {
> +                project.log("Already defined in
> main or a previous import, "
> +                            + "ignore " + name,
> Project.MSG_VERBOSE);
> +            } else {
>                  target.setName(name);
>                
> context.getCurrentTargets().put(name, target);
>                  project.addOrReplaceTarget(name,
> target);
> +                usedTarget = true;
>              }
>
> -            // take care of dependencies
> -            if (depends.length() > 0) {
> -                target.setDepends(depends);
> +            if (context.isIgnoringProjectTag() &&
> context.getCurrentProjectName() != null
> +                &&
> context.getCurrentProjectName().length()
=== message truncated ===



               
Discover Yahoo!
Stay in touch with email, IM, photo sharing and more. Check it out!
http://discover.yahoo.com/stayintouch.html

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [Bug 28444] - Import: Target Handling Bug

Dominique Devienne-2
In reply to this post by peterreilly
> I never liked the target renaming stuff - it seems a bit strange.
> Be that as it may, the current behaviour is a bit silly - i.e. inconsistent.
> The target gets renamed if there another target of the same name, but
> not otherwise - how can one write a proper reusable import file using the
> rename feature in this case?

I never liked target renaming either. Sure, renaming all targets is
more consistent, and I'm +1 for it, but it's still very flawed. The
imported build names should never appear in the importer, we should
have an explicit override attribute (or something) that tells Ant the
target is meant to override a target in an imported build (which fails
the build if it does not override any target), and have a "super"
target to possibly rely on, instead of "renamed" target. And targets
from different imported build files that conflict (multiple
inheritance) should raise an error unless explicitly imported "as"
given by the importer (not the name of the imported project as is the
case now). The "as" name used would then be supplied to the "super"
keyword.

Sorry, I couldn't resist bring all that up again. I know it's water on
the bridge, and that I lost this argument a long time ago, but it
still bugs me.

But eh, I gave my +1 to systematic target renaming ;-) --DD

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Bugzilla or dev-list (was Re: [Bug 28444] - Import: Target Handling Bug)

Stefan Bodewig
In reply to this post by Matt Benson
On Thu, 12 May 2005, Matt Benson <[hidden email]> wrote:

> Here's my comment from the bug since Stefan wants to
> be on-list:  :)

I'll go back to the original thread in a minute, just wanted to
explain why I asked Peter to discuss it on the list.

When I see bugzilla reports that look as if some committer has talen
care of it, I tend to ignore subsequent mails from the report,  This
means I could be missing a discussion thread.

Furthermore I live and breathe in my mail client all day, I hate it
when I have to leave it to respond to a mail.  Some bug tracking
systems can be used by email as well, I even think Bugzilla could, but
not our installation.  If I have to start a brwoser in order to write
the two-character sequence "+1", I may just not do it.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Bugzilla or dev-list (was Re: [Bug 28444] - Import: Target Handling Bug)

Matt Benson

--- Stefan Bodewig <[hidden email]> wrote:

> On Thu, 12 May 2005, Matt Benson
> <[hidden email]> wrote:
>
> > Here's my comment from the bug since Stefan wants
> to
> > be on-list:  :)
>
> I'll go back to the original thread in a minute,
> just wanted to
> explain why I asked Peter to discuss it on the list.

That's fine by me.  I was just kidding around--I do
that a lot but humor, esp. stunted humor like mine,
doesn't always come through in e-mail... ;)

-Matt


__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around
http://mail.yahoo.com 

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [Bug 28444] - Import: Target Handling Bug

Stefan Bodewig
In reply to this post by peterreilly
On Thu, 12 May 2005, Peter Reilly <[hidden email]> wrote:

> The target gets renamed if there another target of the same name,
> but not otherwise - how can one write a proper reusable import file
> using the rename feature in this case?

I'm strongly +1 for at least providing renamed aliases for all
targets, independent on how we achieve that.

> The fix will have a small overhead - the target object needs to be
> cloned and given a a new name, whereas the current code just renames
> the target object.

Do we really need to create a complete clone?  Can't we achieve the
same with a more lazy approach?

Turning Matt's idea around:

(1) Target "foo" is in project "bar".
(2a) There already is a target "foo" from the file that imported
     "bar", use the current code, we are ready, "bar.foo" is there.
(2b) There is no other target "foo" yet.  Create an empty placeholder
     target "bar.foo" that depends on "foo".

     If then later a target "foo" is found in the importing buildfile,
     replace the placeholder "bar.foo" with the initial "foo" target.

Wouldn't that work, stay backwards compatible and hide "bar." whenever
possible?

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [Bug 28444] - Import: Target Handling Bug

Stefan Bodewig
In reply to this post by Dominique Devienne-2
On Thu, 12 May 2005, Dominique Devienne <[hidden email]> wrote:

> The imported build names should never appear in the importer, we
> should have an explicit override attribute (or something) that tells
> Ant the target is meant to override a target in an imported build
> (which fails the build if it does not override any target),

This implies that you'd know all target names of all files you import.
With the ideas of importing files you shipped within jars floating
around, I start to wonder whether this will hold true in the future.

> And targets from different imported build files that conflict
> (multiple inheritance) should raise an error unless explicitly
> imported "as" given by the importer (not the name of the imported
> project as is the case now).

Can't we have this as an addition anyway?

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Bugzilla or dev-list

Stefan Bodewig
In reply to this post by Matt Benson
On Thu, 12 May 2005, Matt Benson <[hidden email]> wrote:

> That's fine by me.

That's how I understood your comment as well, but still felt the need
to explain myself.

> I was just kidding around--I do that a lot but humor, esp. stunted
> humor like mine, doesn't always come through in e-mail... ;)

Welcome to the club.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [Bug 28444] - Import: Target Handling Bug

Matt Benson
In reply to this post by peterreilly
--- Stefan Bodewig <[hidden email]> wrote:
[SNIP]

> Turning Matt's idea around:
>
> (1) Target "foo" is in project "bar".
> (2a) There already is a target "foo" from the file
> that imported
>      "bar", use the current code, we are ready,
> "bar.foo" is there.
> (2b) There is no other target "foo" yet.  Create an
> empty placeholder
>      target "bar.foo" that depends on "foo".
>
>      If then later a target "foo" is found in the
> importing buildfile,
>      replace the placeholder "bar.foo" with the
> initial "foo" target.
>
> Wouldn't that work, stay backwards compatible and
> hide "bar." whenever
> possible?

You confused me with the "later."  Even though this
could theoretically happen via 3rd-party API calls, we
wouldn't be able to detect it, would we?  Our local
targets are known before we actually execute a
top-level (target "") import, right?  So what I take
away from the above is that when there is only one
"foo", the real work lives in "foo" while "bar.foo"
depends on "foo" (my idea turned around).  But say the
importER explicitly depends on bar.foo .  Isn't this
still going to pollute the log in the opposite way my
implementation would? :) i.e.

[foo]:

[bar.foo]:

-Matt
>
> Stefan
>
>
---------------------------------------------------------------------
> To unsubscribe, e-mail:
> [hidden email]
> For additional commands, e-mail:
> [hidden email]
>
>



               
Discover Yahoo!
Find restaurants, movies, travel and more fun for the weekend. Check it out!
http://discover.yahoo.com/weekend.html 


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

RE: [Bug 28444] - Import: Target Handling Bug

Phil W-2
I missed the beginning of this thread but just want to say that I personally think that import is the best feature in Ant today (apart from Ant's being in the first place, that is)!
 
Phil :n)

        -----Original Message-----
        From: Matt Benson [mailto:[hidden email]]
        Sent: Thu 12/05/2005 21:43
        To: Ant Developers List
        Cc:
        Subject: Re: [Bug 28444] - Import: Target Handling Bug
       
       

        --- Stefan Bodewig <[hidden email]> wrote:
        [SNIP]
        > Turning Matt's idea around:
        >
        > (1) Target "foo" is in project "bar".
        > (2a) There already is a target "foo" from the file
        > that imported
        >      "bar", use the current code, we are ready,
        > "bar.foo" is there.
        > (2b) There is no other target "foo" yet.  Create an
        > empty placeholder
        >      target "bar.foo" that depends on "foo".
        >
        >      If then later a target "foo" is found in the
        > importing buildfile,
        >      replace the placeholder "bar.foo" with the
        > initial "foo" target.
        >
        > Wouldn't that work, stay backwards compatible and
        > hide "bar." whenever
        > possible?
       
        You confused me with the "later."  Even though this
        could theoretically happen via 3rd-party API calls, we
        wouldn't be able to detect it, would we?  Our local
        targets are known before we actually execute a
        top-level (target "") import, right?  So what I take
        away from the above is that when there is only one
        "foo", the real work lives in "foo" while "bar.foo"
        depends on "foo" (my idea turned around).  But say the
        importER explicitly depends on bar.foo .  Isn't this
        still going to pollute the log in the opposite way my
        implementation would? :) i.e.
       
        [foo]:
       
        [bar.foo]:
       
        -Matt
        >
        > Stefan
        >
        >
        ---------------------------------------------------------------------
        > To unsubscribe, e-mail:
        > [hidden email]
        > For additional commands, e-mail:
        > [hidden email]
        >
        >
       
       
       
                       
        Discover Yahoo!
        Find restaurants, movies, travel and more fun for the weekend. Check it out!
        http://discover.yahoo.com/weekend.html
       
       
        ---------------------------------------------------------------------
        To unsubscribe, e-mail: [hidden email]
        For additional commands, e-mail: [hidden email]
       
       


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: [Bug 28444] - Import: Target Handling Bug

Nicola Ken Barozzi
In reply to this post by Stefan Bodewig
Stefan Bodewig wrote:
> On Thu, 12 May 2005, Dominique Devienne <[hidden email]> wrote:
...
>>And targets from different imported build files that conflict
>>(multiple inheritance) should raise an error unless explicitly
>>imported "as" given by the importer (not the name of the imported
>>project as is the case now).
>
> Can't we have this as an addition anyway?

Why?

--
Nicola Ken Barozzi                   [hidden email]
             - verba volant, scripta manent -
    (discussions get forgotten, just code remains)
---------------------------------------------------------------------


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [Bug 28444] - Import: Target Handling Bug

Stefan Bodewig
In reply to this post by Matt Benson
On Thu, 12 May 2005, Matt Benson <[hidden email]> wrote:

> You confused me with the "later."

I wasn't sure how the target overriding was implemented so wanted to
be save.

> Our local targets are known before we actually execute a top-level
> (target "") import, right?

Probably.  If you say so it will be correct 8-)

> So what I take away from the above is that when there is only one
> "foo", the real work lives in "foo" while "bar.foo" depends on "foo"
> (my idea turned around).

Yes.

> But say the importER explicitly depends on bar.foo .  Isn't this
> still going to pollute the log in the opposite way my implementation
> would? :) i.e.
>
> [foo]:
>
> [bar.foo]:

Yes.  But this is less likely than having the importer depend on "foo"
IMHO.  So in the normal case everything would look the same as today
and in border cases we'll get an additional empty target.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [Bug 28444] - Import: Target Handling Bug

Phil W-2
On Fri, 2005-05-13 at 09:00 +0200, Stefan Bodewig wrote:

> > But say the importER explicitly depends on bar.foo .  Isn't this
> > still going to pollute the log in the opposite way my implementation
> > would? :) i.e.
> >
> > [foo]:
> >
> > [bar.foo]:
>
> Yes.  But this is less likely than having the importer depend on "foo"
> IMHO.  So in the normal case everything would look the same as today
> and in border cases we'll get an additional empty target.

If I understand what you're saying correctly, you don't expect importer
build scripts to depend explicitly on the renamed imported targets. I'd
disagree: we commonly "augment" the standard targets like this:

standard.xml:

<project name="standard">
    <target name="compile">...</target>
</project>

build.xml:

<project name="my-build" basedir="." default="compile">
    <import file="${standard.dir}/standard.xml"/>

    <target name="compile" depends="standard.compile">...</target>
</project>

I'm not sure if this is what you're discussing or not. What I can say is
that the output of targets and tasks used in our build scripts is very
unclear.

We generally find it difficult tell actually which target caused a given
dependency target to be executed because there are too many "empty"
target names being output (we have various targets that operate "if" and
"unless" variants, but the target name is output even if the condition
fails). In addition, the same "standard" targets are executed multiple
times during our build in different contexts since we have the concept
of "subsystems" within our build:

main-dir/build.xml "ant" executes:

main-dir/subsystem-one/build.xml
main-dir/subsystem-two/build.xml
main-dir/subsystem-.../build.xml

from within targets that have dependencies between each other that
describe subsystem dependencies. Each subsystem build.xml imports the
standard targets and executes them in its context.

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [Bug 28444] - Import: Target Handling Bug

Steve Loughran
In reply to this post by Phil W-2
Phil Weighill-Smith wrote:
> I missed the beginning of this thread but just want to say that I personally think that import is the best feature in Ant today (apart from Ant's being in the first place, that is)!

I find it hard to work with in a big project.

problems
-risk of adding a new target in an imported build file conflicting with
one in the importer (i.e. lack of private scope)

-when you override a target, you dont get access to its dependents.
Workaround: many pseudo-targets that only model dependencies.

-once you have sub-projects importing ../../common.xml, they are no
longer self contained, which makes it harder to work with outside the
existing build tree.

I dont see any good solution for the latter.
-steve

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [Bug 28444] - Import: Target Handling Bug

Nicola Ken Barozzi
Steve Loughran wrote:

> Phil Weighill-Smith wrote:
>
>> I missed the beginning of this thread but just want to say that I
>> personally think that import is the best feature in Ant today (apart
>> from Ant's being in the first place, that is)!
>
> I find it hard to work with in a big project.
>
> problems
> -risk of adding a new target in an imported build file conflicting with
> one in the importer (i.e. lack of private scope)
 >
> -when you override a target, you dont get access to its dependents.
> Workaround: many pseudo-targets that only model dependencies.

Could you please explain more? TIA

> -once you have sub-projects importing ../../common.xml, they are no
> longer self contained, which makes it harder to work with outside the
> existing build tree.
>
> I dont see any good solution for the latter.

The first and the last "problems" are in fact worksforme, as the import
task is *intended* to pollute the project files that use it to be able
to work as needed.

In most cases, one would want to use <typedef>, <macrodef> or <ant>
instead of importing buildfiles with <target>s, which should be used
only to reuse and "extend" a base buildfile.

--
Nicola Ken Barozzi                   [hidden email]
             - verba volant, scripta manent -
    (discussions get forgotten, just code remains)
---------------------------------------------------------------------


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [Bug 28444] - Import: Target Handling Bug

Stefan Bodewig
In reply to this post by Steve Loughran
On Fri, 13 May 2005, Steve Loughran <[hidden email]> wrote:

> Phil Weighill-Smith wrote:
>> I missed the beginning of this thread but just want to say that I
>> personally think that import is the best feature in Ant today
>> (apart from Ant's being in the first place, that is)!
>
> I find it hard to work with in a big project.
>
> problems
> -risk of adding a new target in an imported build file conflicting
> with one in the importer (i.e. lack of private scope)

Does that really happen for you?

The workaround is to use ugly, likely to be unique target names - or
address the enhancement request this discussion came from and depend
on "imported.foo" for your newly introduced meant-to-be-private target
"foo".

> -when you override a target, you dont get access to its
> dependents. Workaround: many pseudo-targets that only model
> dependencies.

When you completely want to replace a target that is, true.

Another related problem I have is that you can't add something to the
beginning of a target easily.  You can add someting to the end of
"foo" by writing your own "foo" that depends on "imported.foo", but if
you want to add something at the start your imported build file has to
be designed for it.  Which means pseudo-targets that are used as
interception points only.

> -once you have sub-projects importing ../../common.xml, they are no
> longer self contained, which makes it harder to work with outside
> the existing build tree.

Once we can import URLs, you can have a solution for this, at least
for "network connected" development.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [Bug 28444] - Import: Target Handling Bug

Stefan Bodewig
In reply to this post by Phil W-2
On Fri, 13 May 2005, Phil Weighill Smith
<[hidden email]> wrote:

> On Fri, 2005-05-13 at 09:00 +0200, Stefan Bodewig wrote:
>
>> > But say the importER explicitly depends on bar.foo .  Isn't this
>> > still going to pollute the log in the opposite way my
>> > implementation would? :) i.e.
>> >
>> > [foo]:
>> >
>> > [bar.foo]:
>>
>> Yes.  But this is less likely than having the importer depend on
>> "foo" IMHO.  So in the normal case everything would look the same
>> as today and in border cases we'll get an additional empty target.
>
> If I understand what you're saying correctly, you don't expect
> importer build scripts to depend explicitly on the renamed imported
> targets.

No.  I fully expect importers to do so now.  And if they do they must
have overridden the target themselves, so what Matt descibes doesn't
happen.

Let's take your example and add a target.

> I'd disagree: we commonly "augment" the standard targets like this:
>
> standard.xml:
>
> <project name="standard">
>     <target name="compile">...</target>
      <target name="new-target"/>
> </project>
>
> build.xml:
>
> <project name="my-build" basedir="." default="compile">
>     <import file="${standard.dir}/standard.xml"/>
>
>     <target name="compile" depends="standard.compile">...</target>
> </project>

Today your build.xml can't use a target that depends on
"standard.new-target" as this target doesn't exist.

Peter's approach would create a copy of "new-target" with the name
"standard.new-target".

Matt's approach would implicitly add a new target

  <target name="new-target" depends="standard.new-target"/>

to build.xml (which also renames the "new-target" in standard.xml to
"standard.new-target").  Now if you run

$ ant new-target

the output will be

,----
| standard.new-target:
|
| new-target:
`----

and you see that new-target has been imported from standard.

My approach would adds

  <target name="standard.new-target" depends="new-target"/>

to build.xml.  "ant new-target" still looks the same as it would if we
don't add a target at all.  But now if you use

$ ant standard.new-target

the output will be

,----
| new-target:
|
| standard.new-target:
`----

So in either case you get to see an implementation detail - the empty
target that either Matt or I wanted to introduce.  I simply claimed
that Matt's case was more likely to happen than mine since you can't
depend on "standard.new-target" or invoke "ant standard.new-target" at
all today.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [Bug 28444] - Import: Target Handling Bug

Stefan Bodewig
In reply to this post by Nicola Ken Barozzi
On Fri, 13 May 2005, Nicola Ken Barozzi <[hidden email]> wrote:

> Stefan Bodewig wrote:
>> On Thu, 12 May 2005, Dominique Devienne <[hidden email]>
>> wrote:
> ...
>>>And targets from different imported build files that conflict
>>>(multiple inheritance) should raise an error unless explicitly
>>>imported "as" given by the importer (not the name of the imported
>>>project as is the case now).
>> Can't we have this as an addition anyway?
>
> Why?

To give the importer control over the prefix and make it a bit more
independent from the author of the imported file.  I may even want to
import to files coming from two different authors but having the same
project name.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

123