Bug 28444 - Import: Target Handling Bug
Summary: Import: Target Handling Bug
Status: RESOLVED FIXED
Alias: None
Product: Ant
Classification: Unclassified
Component: Core tasks (show other bugs)
Version: 1.6.1
Hardware: PC other
: P3 major with 1 vote (vote)
Target Milestone: ---
Assignee: Ant Notifications List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-04-17 00:40 UTC by Mike Murray
Modified: 2008-02-22 12:18 UTC (History)
1 user (show)



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Murray 2004-04-17 00:40:06 UTC
Because imported targets are only qualified by their owning project name when a
target by that name has already been defined, it can result in a non-existent
target error in certain contexts.

If project B & C both import A, and both A & B define target x, and B.x defines
A.x as its depends, it will fail due to a non-existent A.x target for C, unless
C also defines it.

<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'.
Comment 1 Matt Benson 2004-04-17 19:32:20 UTC
Mike:  I have been playing around with similar concepts for days... however, in 
this case, knowing that B imports A, C would merely import B.  Or, had you 
imported B before A, thus defining x first, all would have been well.  OR, had 
C defined x, that would have been okay too.  I also thought it might be nice if 
there were a way to specify that imported targets would always be added with 
their "super" name, but I am undecided whether it's an entirely good idea... 
guess we'll see if Peter or anyone else has an opinion on this one...
Comment 2 Mike Murray 2004-04-18 19:51:49 UTC
I am aware that there are work arounds for the simple scenario that I provided,
such as managing the order of the imports and redefining the target in the file
where the problem arises.  Redefining targets merely to ensure consistent names
is undesirable and in a complex hierarchy of imports, managing the order might
not be a solution at all.

In any case, the inconsistency would not need to be managed if Ant always
qualified the target name with the name of the project, whenever it "could" be
needed.
Comment 3 Dominique Devienne 2004-04-19 14:16:28 UTC
ALAS, what you guys are describing results from the use of the project names to 
prefix the targets. I was SQUARELY against such a bad practice, which breaks 
encapsulation.

With the model I proposed, no renaming took place, unless explicitly specified 
by the importER build, and the prefix used is local/private to the importER 
build. And I mean really private, i.e. somebody else importing that importER 
build never sees the prefixes used.

This removes all ambiguities one build file at a time, without affecting 
anybody potentially using/importing that build down the line.

At least I think this would have been a better scheme ;-) You guys are welcome 
to find edge cases that shows it wouldn't have ;-))). --DD
Comment 4 Evan Easton 2004-04-19 14:40:42 UTC
I really don't think that allowing developers to qualify targets with their 
declaring project names is "bad practice."  Most languages that support a 
notion of override give you this fine degree of control.  Obviously, using a 
fully qualified target (<project>.<target>) when the unqualified one is 
sufficient isn't recommended (polymorphism is good).  But not leaving a 
developer with a consistent mechanism to ensure that the base/overridable 
target is always called in certain cases is a problem.

I'm not certain that we have to always make all targets referencable with the 
project name.  I'm sure this has performance implications.  So as DD suggests 
an import attribute that allows the importER to control the behavior sounds 
reasonable.
Comment 5 Dominique Devienne 2004-04-19 14:48:26 UTC
I'm not sure given the last post my point came across. I'm not against using 
using a prefix to resolve conflicts between imported targets and/or with the 
local importER's targets. I'm against using the importEE's project name as the 
prefix.

I can't import two build files (without modifying them) with the 'common' 
project name for examples. And it opens the door to weird edge cases in import. 
What I propose is that the import prefix be made optional (no need for it if 
there are no conflicts), and when necessary to disambiguate conflicts, the 
importER decide what prefixes to use in its OWN build script, and that 
additionally the prefixes used are completely private to the importER, i.e. an 
implementation detail to the importER.

Anyways, this battle was already lost, and the current behavior is here to 
stay. Not that it's that bad really, just poor design in my book. But hey, I'm 
surely wrong since I've been overriden by everyone. --DD
Comment 6 Matt Benson 2004-04-19 15:03:12 UTC
Dominique, when you can, could you find the relevant thread?  I'd like to see 
what the con arguments were; seems to me it's still not too late to add one or 
more attributes to <import> to modify the current behavior...
Comment 7 Jose Alberto Fernandez 2004-04-20 13:24:03 UTC
Dominique, I think I was bouching for a different approach (XSLT like) at the 
time which also lost. In any case, under the current approach I tend to agree 
with you.

What I would suggest we can do now is the following:

1) Add a new optional attribute (e.g., "as") which declares a local (private) 
name for the import. The value of this attribute is the only name available to
refer to the imported targets.

 <import file="A.xml" as="myLocalA"/>

2) The default value for the "as" attribute is the name of the <project/> in 
the imported file. (That provides backward compatibility with the current 
behaviour).

Not sure if this simple rules are enough. I would really would like to keep it 
as simple as possible. And most of all avoid aliasing (i.e., the same target 
being accessible by two or more different full names.
Comment 8 Austin Hastings 2005-02-01 17:34:18 UTC
Adding an "optional if no conflict" prefix is insufficient. My scenario is a tad
different: I've got a 'framework' buildfile that imports local rules. I require
the local rules to define certain targets, so I know that e.g.
"local_rules.clean" exists (if it doesn't exist, an abort is desirable
behavior). There is also a automatically generated portion in a separate file. I
know what the filename/projectname is, so that's fine. 

But when developing a new target-set, not all of the local rules files or
framework files may be totally up-to-date. This only becomes irksome when one
component, that *KNOWS* someproject.sometarget exists, explodes because
"someproject.sometarget" is not defined--although "sometarget" is.

I don't know how the internals of import work, but I cannot imagine that adding
two namespace entries is a performance consideration of any kind. Please make
this behavior consistent.
Comment 9 Peter Reilly 2005-05-12 16:24:47 UTC
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.
Comment 10 Matt Benson 2005-05-12 17:34:18 UTC
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
Comment 11 Alexey Solofnenko 2005-05-12 17:47:47 UTC
+1

That is similar to what I do in my preprocessor.
Comment 12 Peter Reilly 2005-05-17 12:54:20 UTC
Fix checked into cvs HEAD.
The      "ant -f C.xml x" test will now work.