You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ant.apache.org by bu...@apache.org on 2016/02/11 18:35:47 UTC

[Bug 58997] New: Replace randomly fails using properties file

https://bz.apache.org/bugzilla/show_bug.cgi?id=58997

            Bug ID: 58997
           Summary: Replace randomly fails using properties file
           Product: Ant
           Version: 1.9.6
          Hardware: All
                OS: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Core tasks
          Assignee: notifications@ant.apache.org
          Reporter: istvan@verhas.com

Created attachment 33548
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=33548&action=edit
the sample build.xml an properties files to reproduce the issue

If you use a properties file to replace tokens in a file it sometimes fails
when there are two properties that one is a substring of other one. Actually it
is typical in a properties file that you build it hierarchically.
The root cause is that Properties extends HashTable which does not handle az
insertion order. I propose a solution to sort on the length of the token is
descending order.
The patch I tested

--- Replace-orig.java    2015-06-29 07:46:12.000000000 +0100
+++ Replace.java    2016-02-11 18:28:18.974000000 +0100
@@ -30,7 +30,11 @@
 import java.io.OutputStreamWriter;
 import java.io.Reader;
 import java.io.Writer;
+import java.lang.Override;
+import java.util.*;
 import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
 import java.util.Iterator;
 import java.util.Properties;

@@ -67,7 +71,7 @@
     private Resource propertyResource = null;
     private Resource replaceFilterResource = null;
     private Properties properties = null;
-    private ArrayList replacefilters = new ArrayList();
+    private ArrayList<Replace.Replacefilter> replacefilters = new ArrayList();

     private File dir = null;

@@ -523,7 +527,7 @@
                 properties = getProperties(propertyResource);
             }

-            validateReplacefilters();
+            validateAndOrderReplacefilters();
             fileCount = 0;
             replaceCount = 0;

@@ -597,7 +601,7 @@
      * @exception BuildException if any supplied attribute is invalid or any
      * mandatory attribute is missing.
      */
-    public void validateReplacefilters()
+    public void validateAndOrderReplacefilters()
             throws BuildException {
         final int size = replacefilters.size();
         for (int i = 0; i < size; i++) {
@@ -605,6 +609,22 @@
                 (Replacefilter) replacefilters.get(i);
             element.validate();
         }
+        orderReplacefilters();
+    }
+
+    /*
+    patch to handle correctly properties files where the tokens can be
substrings of eachother
+    the Properties is a HashTable that does not care with he insertion order
+    Sometimes it works sometimes not.
+    */
+
+    private void     orderReplacefilters(){
+        Collections.sort(replacefilters, new Comparator<Replacefilter>() {
+            @Override
+            public int compare(Replacefilter rf1, Replacefilter rf2) {
+                return rf2.getToken().length() - rf1.getToken().length();
+            }
+        });
     }

     /**

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58997] Replace randomly fails using properties file

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58997

istvan@verhas.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|FIXED                       |---
             Status|VERIFIED                    |REOPENED

--- Comment #3 from istvan@verhas.com ---
I have to reopen this issue as using the 1.9.7alpha i found that it is
replacing only a part of the provided tokens. Actually i have not enough time
to debug your solution but i tested the solution i proposed. The later is still
working well on the sample 20160217.zip .

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58997] Replace randomly fails using properties file

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58997

Stefan Bodewig <bo...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|REOPENED                    |RESOLVED

--- Comment #6 from Stefan Bodewig <bo...@apache.org> ---
should be fixed with git 5a6fda5 - really :-)

Many thanks for re-testing my initial attempt.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58997] Replace randomly fails using properties file

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58997

istvan@verhas.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |istvan@verhas.com

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58997] Replace randomly fails using properties file

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58997

istvan@verhas.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |VERIFIED

--- Comment #2 from istvan@verhas.com ---
The backward compatibility note is correct. 
I verified the solution on the sample and it fixed the issue.
thanks.
bye
steve

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58997] Replace randomly fails using properties file

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58997

--- Comment #4 from istvan@verhas.com ---
Created attachment 33564
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=33564&action=edit
extended sample build.xml an properties files to reproduce the issue

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58997] Replace randomly fails using properties file

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58997

Stefan Bodewig <bo...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED
   Target Milestone|---                         |1.9.7

--- Comment #1 from Stefan Bodewig <bo...@apache.org> ---
The change you propose would break backwards compatibility. People may rely on
the order of nested replacefilter elements.

Git commit 4d0bbb4 contains a sligtly different approach inspired by yours that
only sorts the filters drawn from a replacefilterfile. It would be good if you
could verify it fixes the problem for you.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 58997] Replace randomly fails using properties file

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58997

--- Comment #5 from Stefan Bodewig <bo...@apache.org> ---
You are correct, my fault. For the TreeSet two properties are now equal if
they've got the same length.

I'll fix it.

-- 
You are receiving this mail because:
You are the assignee for the bug.