Issue with detecting if it's a platform context

[Mandy Chung]

On 06/02/10 21:34, mark.reinhold at oracle.com wrote:
>> 2. The optional modules are not linked and recorded in the config.  The fix is
>> to skip the optional modules in the Resolver only if it doesn't exist.
>>     
>
> The resolver fix for optional modules needs adjustment.  You've got
> the right idea in moving the Modifier.OPTIONAL test further down in
> the resolve(int, Choice) method.  Placing it at the end of the method,
> however, means that the resolver will consider remote, not-yet-installed
> modules as candidates for satisfying optional dependences, and that's
> not the right thing.  The OPTIONAL test should be placed just before the
> section that checks remote repositories.  I'd also change the comment to
> say "Don't fail; it's just an optional dependence".
>   

Thanks for catching it.  Here is the diff:

diff --git a/src/share/classes/org/openjdk/jigsaw/Resolver.java 
b/src/share/classes/org/openjdk/jigsaw/Resolver.java
--- a/src/share/classes/org/openjdk/jigsaw/Resolver.java
+++ b/src/share/classes/org/openjdk/jigsaw/Resolver.java
@@ -48,7 +48,6 @@ import static org.openjdk.jigsaw.Trace.*
 // of the dependence graph since different versions of the same module can
 // have completely different dependences.

-// ## TODO: Implement optional dependences
 // ## TODO: Implement provides
 // ## TODO: Improve error messages

@@ -176,6 +175,11 @@ final class Resolver {
                 return true;
         }

+        if (dep.modifiers().contains(Modifier.OPTIONAL)) {
+            // Don't fail; it's just an optional dependence
+            return resolve(depth + 1, choice.next);
+        }
+
         // No local module found, so if this catalog is a library then
         // consider its remote repositories, if any
         //
@@ -199,11 +203,6 @@ final class Resolver {
                         return true;
                 }
             }
-        }
-
-        if (dep.modifiers().contains(Modifier.OPTIONAL)) {
-            // Skip for now; we'll handle these later
-            return resolve(depth + 1, choice.next);
         }

         if (tracing)

> Oh, and please extend the _Configurator test to check that optional
> dependences now work correctly.  There are a couple of commented-out
> tests which can serve as a starting point.
>
>   

Will do that.  Thanks for the review.

Mandy