A compromise approach on circularity checking

[Remi Forax]

Hi Davidn
thanks for keep us focus on what are the issues we should discuss.


I agree with this proposal, detecting cycle by default will raise awareness that there is a mismatch beetween the configuration at compile time and the configuration at runtime so it should be enabled by default and i agree that from a pragmatic point of view, you can disable it because this is the current behavior and it will help people to transitioning to JPMS.

Rémi

----- Mail original -----
> De: "David M. Lloyd" <david.lloyd at redhat.com>
> À: jpms-spec-experts at openjdk.java.net
> Envoyé: Vendredi 12 Mai 2017 02:18:26
> Objet: A compromise approach on circularity checking

> I'd like to propose a patch [1] which allows run-time circularity
> checking to be disabled on an opt-out basis.  This allows an approach
> wherein data can be collected as to the usefulness of circularity
> checking, without locking users into it (at least in the near term).
> 
> Justification:
> 
> • In many environments, including large enterprises, or large open- and
> closed-source projects which may consume or assemble many dependencies,
> it is possible that a module which builds without cycles may fail due to
> a cycle at assembly time.
> • By making this change opt-out, most users need not worry about it; if
> assembly problems occur at the end of the development or test process,
> however, users have an "escape hatch" at their disposal.
> • If it becomes clear that allowing cycles at run time is indeed a valid
> requirement, then cycle checking at run time can be disabled permanently
> in Java 10 or a later release.
> 
> Drawbacks:
> 
> None that I am aware of.
> 
> Patch information:
> 
> The approach of this patch simply looks for a system property and,
> depending on the value of this property, enables or disables cycle
> checking.  I relinquish any copyright claim to the patch in this mail.
> Please do not get bogged down by any formatting problems introduced by
> the mailing list; the purpose of directly including the patch is to give
> a clear, unambiguous subject for discussion.  I can provide a proper
> webrev (or whatever other form is requested) if needed.
> 
> Other than the system property (which is just one possible approach), no
> API or specification changes should be needed.  This version uses the
> property "jdk.module.detect-cycles" defaulting to "true".
> 
> [1]:
> 
> diff --git
> a/jdk/src/java.base/share/classes/java/lang/module/Resolver.java
> b/jdk/src/java.base/share/classes/java/lang/module/Resolver.java
> index a723e63..2451358 100644
> --- a/jdk/src/java.base/share/classes/java/lang/module/Resolver.java
> +++ b/jdk/src/java.base/share/classes/java/lang/module/Resolver.java
> @@ -351,6 +351,8 @@ final class Resolver {
>          }
>      }
> 
> +    private static final boolean DETECT_CYCLES =
> Boolean.parseBoolean(System.getProperty("jdk.module.detect-cycles",
> "true"));
> +
>      /**
>       * Execute post-resolution checks and returns the module graph of
> resolved
>       * modules as {@code Map}. The resolved modules will be in the given
> @@ -362,7 +364,7 @@ final class Resolver {
>                                                      boolean check)
>      {
>          if (check) {
> -            detectCycles();
> +            if (DETECT_CYCLES) detectCycles();
>              checkHashes();
>          }
> 
> 
> 
> --
> - DML