Realized that I hadn't tested negative qualfiers, added tests, discovered that

they failed. Fixed problems and added sanity checks to ensure that negative
qualifiers are always used alone.

src/main/java/com/threerings/getdown/util/ConfigUtil.java

@@ -21,6 +21,7 @@
 // INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
 // LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
 // SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
 package com.threerings.getdown.util;
 
 import java.io.File;
@@ -120,9 +121,7 @@ public class ConfigUtil
         }
     }
 
-    /**
+    /** A helper function for {@link #parsePairs(Reader,boolean}. */
-     * A helper function for {@link #parsePairs(Reader,boolean}.
-     */
     protected static List<String[]> parsePairs (Reader config, String osname, String osarch)
         throws IOException
     {
@@ -183,26 +182,33 @@ public class ConfigUtil
      * ids = id | id,ids
      * quals = !id | ids
      * </pre>
-     * Examples: [linux-amd64,linux-x86_64], [windows], [mac os x], [!windows].
+     * Examples: [linux-amd64,linux-x86_64], [windows], [mac os x], [!windows]. Negative qualifiers
+     * must only be used alone as they trigger match or non-match immediately, irrespective of the
+     * value of other qualifiers.
      */
     protected static boolean checkQualifiers (String quals, String osname, String osarch)
     {
+        if (quals.startsWith("!")) {
+            if (quals.indexOf(",") != -1) { // sanity check
+                log.warning("Multiple qualifiers cannot be used when one of the qualifiers " +
+                            "is negative", "qual", quals);
+                return false;
+            }
+            return !checkQualifier(quals.substring(1), osname, osarch);
+        }
         for (String qual : quals.split(",")) {
-            String[] bits = qual.trim().toLowerCase().split("-");
+            if (checkQualifier(qual, osname, osarch)) {
-            String os = bits[0], arch = (bits.length > 1) ? bits[1] : "";
+                return true; // if we have a positive match, we can immediately return true
-            if (os.startsWith("!")) {
-                // if we have a negative match, we can immediately return false
-                if (osname.indexOf(os.substring(1)) != -1 && osarch.indexOf(arch) != -1) {
-                    return false;
-                }
-            } else {
-                // if we have a positive match, we can immediately return true
-                if (osname.indexOf(os) != -1 && osarch.indexOf(arch) != -1) {
-                    return true;
-                }
             }
         }
-        // we had no positive matches, so return false
+        return false; // we had no positive matches, so return false
-        return false;
+    }
+
+    /** A helper function for {@link #checkQualifiers}. */
+    protected static boolean checkQualifier (String qual, String osname, String osarch)
+    {
+        String[] bits = qual.trim().toLowerCase().split("-");
+        String os = bits[0], arch = (bits.length > 1) ? bits[1] : "";
+        return (osname.indexOf(os) != -1) && (osarch.indexOf(arch) != -1);
     }
 }

src/test/java/com/threerings/getdown/util/ConfigUtilTest.java

@@ -74,7 +74,8 @@ public class ConfigUtilTest
         Pair linux64s = new Pair("nine", "[linux-x86_64, linux-amd64] ten");
         Pair mac64 = new Pair("eleven", "[mac os x-x86_64] twelve");
         Pair win64 = new Pair("thirteen", "[windows-x86_64] fourteen");
-        Pair[] pairs = { linux, mac, linuxAndMac, linux64, linux64s, mac64, win64 };
+        Pair notWin = new Pair("fifteen", "[!windows] sixteen");
+        Pair[] pairs = { linux, mac, linuxAndMac, linux64, linux64s, mac64, win64, notWin };
 
         List<String[]> parsed = ConfigUtil.parsePairs(toReader(pairs), "linux", "i386");
         assertTrue(exists(parsed, linux.key));
@@ -84,6 +85,7 @@ public class ConfigUtilTest
         assertTrue(!exists(parsed, linux64s.key));
         assertTrue(!exists(parsed, mac64.key));
         assertTrue(!exists(parsed, win64.key));
+        assertTrue(exists(parsed, notWin.key));
 
         parsed = ConfigUtil.parsePairs(toReader(pairs), "linux", "x86_64");
         assertTrue(exists(parsed, linux.key));
@@ -93,6 +95,7 @@ public class ConfigUtilTest
         assertTrue(exists(parsed, linux64s.key));
         assertTrue(!exists(parsed, mac64.key));
         assertTrue(!exists(parsed, win64.key));
+        assertTrue(exists(parsed, notWin.key));
 
         parsed = ConfigUtil.parsePairs(toReader(pairs), "linux", "amd64");
         assertTrue(exists(parsed, linux.key));
@@ -102,6 +105,7 @@ public class ConfigUtilTest
         assertTrue(exists(parsed, linux64s.key));
         assertTrue(!exists(parsed, mac64.key));
         assertTrue(!exists(parsed, win64.key));
+        assertTrue(exists(parsed, notWin.key));
 
         parsed = ConfigUtil.parsePairs(toReader(pairs), "mac os x", "x86_64");
         assertTrue(!exists(parsed, linux.key));
@@ -111,6 +115,7 @@ public class ConfigUtilTest
         assertTrue(!exists(parsed, linux64s.key));
         assertTrue(exists(parsed, mac64.key));
         assertTrue(!exists(parsed, win64.key));
+        assertTrue(exists(parsed, notWin.key));
 
         parsed = ConfigUtil.parsePairs(toReader(pairs), "windows", "i386");
         assertTrue(!exists(parsed, linux.key));
@@ -120,6 +125,7 @@ public class ConfigUtilTest
         assertTrue(!exists(parsed, linux64s.key));
         assertTrue(!exists(parsed, mac64.key));
         assertTrue(!exists(parsed, win64.key));
+        assertTrue(!exists(parsed, notWin.key));
 
         parsed = ConfigUtil.parsePairs(toReader(pairs), "windows", "x86_64");
         assertTrue(!exists(parsed, linux.key));
@@ -129,6 +135,7 @@ public class ConfigUtilTest
         assertTrue(!exists(parsed, linux64s.key));
         assertTrue(!exists(parsed, mac64.key));
         assertTrue(exists(parsed, win64.key));
+        assertTrue(!exists(parsed, notWin.key));
 
         parsed = ConfigUtil.parsePairs(toReader(pairs), "windows", "amd64");
         assertTrue(!exists(parsed, linux.key));
@@ -138,6 +145,7 @@ public class ConfigUtilTest
         assertTrue(!exists(parsed, linux64s.key));
         assertTrue(!exists(parsed, mac64.key));
         assertTrue(!exists(parsed, win64.key));
+        assertTrue(!exists(parsed, notWin.key));
     }
 
     protected static boolean exists (List<String[]> pairs, String key)