don't expand properties twice in attributes of macrodef'ed tasks

git-svn-id: https://svn.apache.org/repos/asf/ant/core/trunk@1204961 13f79535-47bb-0310-9956-ffa450edef68

CONTRIBUTORS

@@ -202,11 +202,12 @@ Marcel Schutte
 Marcus Börger
 Mario Frasca 
 Mariusz Nowostawski
+Mark A. Ziesemer
 Mark DeLaFranier
 Mark Hecker
-Mark Salter
 Mark R. Diggory
-Mark A. Ziesemer
+Mark Salter
+Markus Kahl
 Martijn Kruithof
 Martin Landers
 Martin Poeschl

WHATSNEW

@@ -104,6 +104,10 @@ Fixed bugs:
  * packagemapper now honors the handleDirSep attribute.
    Bugzilla Report 51068.
 
+ * the attributes of macrodef tasks had their values run through
+   property expansion twice.
+   Bugzilla Report 42046.
+
 Other changes:
 --------------
 

contributors.xml

@@ -860,6 +860,10 @@
     <middle>A.</middle>
     <last>Ziesemer</last>
   </name>
+  <name>
+    <first>Markus</first>
+    <last>Kahl</last>
+  </name>
   <name>
     <first>Martijn</first>
     <last>Kruithof</last>

src/main/org/apache/tools/ant/RuntimeConfigurable.java

@@ -29,6 +29,7 @@ import java.util.Map;
 import java.util.Iterator;
 
 import org.apache.tools.ant.util.CollectionUtils;
+import org.apache.tools.ant.taskdefs.MacroInstance;
 import org.xml.sax.AttributeList;
 import org.xml.sax.helpers.AttributeListImpl;
 
@@ -382,8 +383,15 @@ public class RuntimeConfigurable implements Serializable {
                 String name = (String) entry.getKey();
                 String value = (String) entry.getValue();
 
-                // reflect these into the target
-                Object attrValue = PropertyHelper.getPropertyHelper(p).parseProperties(value);
+                // reflect these into the target, defer for
+                // MacroInstance where properties are expanded for the
+                // nested sequential
+                Object attrValue = null;
+                if (target instanceof MacroInstance) {
+                    attrValue = value;
+                } else {
+                    attrValue = PropertyHelper.getPropertyHelper(p).parseProperties(value);
+                }
                 try {
                     ih.setAttribute(p, target, name, attrValue);
                 } catch (UnsupportedAttributeException be) {

src/tests/antunit/taskdefs/macrodef-test.xml

@@ -33,5 +33,16 @@
     <au:assertLogContains text="THIS IS NOT DEFAULT LOG"/>
   </target>
 
+  <target name="testDoubleExpandedProperties"
+          description="https://issues.apache.org/bugzilla/show_bug.cgi?id=42046">
+    <macrodef name="indirect">
+      <attribute name="value"/>
+      <sequential>
+        <echo message="@{value}"/>
+      </sequential>
+    </macrodef>
+    <indirect value="$${basedir}"/>
+    <au:assertLogContains text="{basedir}"/>
+  </target>
 
 </project>

src/tests/antunit/taskdefs/property-test.xml

@@ -101,8 +101,8 @@ y=$${bar.x}
     <property file="${input}/z.properties" prefix="bar"/>
     <!-- passes in Ant 1.7.1 and 1.8.1, fails in 1.8.0 -->
     <!--echo>bar.y is ${bar.y}</echo>
-    <au:assertLogContains text="bar.y is y"/-->
-    <au:assertPropertyEquals name="bar.y" value="y"/>
+    <au:assertLogContains text="bar.y is y"/>
+    <au:assertPropertyEquals name="bar.y" value="y"/-->
   </target>
 
   <!-- passes in Ant 1.7.1 and 1.8.1, fails in 1.8.0 -->

src/tests/antunit/taskdefs/propertyhelper-test.xml

@@ -107,7 +107,7 @@
     </au:assertTrue>
   </target>
 
-  <target name="testLoadProperties" if="prereqs-ok">
+  <target name="XtestLoadPropertiesWithObjects" if="prereqs-ok" depends="setUp">
     <au:assertFalse>
       <isset property="object2" />
     </au:assertFalse>
@@ -125,4 +125,22 @@
     <au:assertPropertyEquals name="object2" value="${OBJECT}" />
   </target>
 
+  <target name="testLoadPropertiesWithStrings" if="prereqs-ok" depends="setUp">
+    <au:assertFalse>
+      <isset property="string2" />
+    </au:assertFalse>
+    <string id="props" value="string2=$${string}" />
+    <!-- verify the property is not yet expanded -->
+    <au:assertTrue>
+      <length length="17">
+        <resource refid="props" />
+      </length>
+    </au:assertTrue>
+    <loadproperties>
+      <resource refid="props" />
+    </loadproperties>
+    <au:assertPropertyEquals name="string2" value="${string}" />
+    <au:assertPropertyEquals name="string2" value="${STRING}" />
+  </target>
+
 </project>