Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

CeylonModuleLoader: fix for #31: automagically import/export jdk modu…

…le with every jdk package
  • Loading branch information...
commit 4079311c3a25fdbac399ff002946b62f8fea2348 1 parent 9951595
Stéphane Épardaud FroMage authored
73 impl/src/main/java/ceylon/modules/jboss/runtime/CeylonModuleLoader.java
View
@@ -19,22 +19,12 @@
import java.io.File;
import java.util.ArrayList;
-import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
-import ceylon.modules.api.util.ModuleVersion;
-import ceylon.modules.jboss.repository.ResourceLoaderProvider;
-import com.redhat.ceylon.cmr.api.ArtifactContext;
-import com.redhat.ceylon.cmr.api.ArtifactResult;
-import com.redhat.ceylon.cmr.api.ImportType;
-import com.redhat.ceylon.cmr.api.JDKUtils;
-import com.redhat.ceylon.cmr.api.RepositoryManager;
-import com.redhat.ceylon.cmr.api.VisibilityType;
-import com.redhat.ceylon.common.Versions;
import org.jboss.modules.DependencySpec;
import org.jboss.modules.LocalLoader;
import org.jboss.modules.ModuleIdentifier;
@@ -45,6 +35,16 @@
import org.jboss.modules.ResourceLoaderSpec;
import org.jboss.modules.filter.PathFilters;
+import ceylon.modules.api.util.ModuleVersion;
+import ceylon.modules.jboss.repository.ResourceLoaderProvider;
+
+import com.redhat.ceylon.cmr.api.ArtifactContext;
+import com.redhat.ceylon.cmr.api.ArtifactResult;
+import com.redhat.ceylon.cmr.api.ImportType;
+import com.redhat.ceylon.cmr.api.JDKUtils;
+import com.redhat.ceylon.cmr.api.RepositoryManager;
+import com.redhat.ceylon.common.Versions;
+
/**
* Ceylon JBoss Module loader.
* It understands Ceylon repository notion.
@@ -65,7 +65,8 @@
private static final String CEYLON_RUNTIME_PATH;
private static final Set<ModuleIdentifier> BOOTSTRAP;
- private static final Map<String, DependencySpec> SYSTEM_DEPENDENCIES;
+ private static final DependencySpec JDK_DEPENDENCY;
+ private static final Set<String> JDK_MODULE_NAMES;
static {
final String defaultVersion = System.getProperty("ceylon.version", Versions.CEYLON_VERSION_NUMBER);
@@ -92,19 +93,22 @@
BOOTSTRAP.add(JANDEX);
BOOTSTRAP.add(RUNTIME);
- SYSTEM_DEPENDENCIES = new HashMap<String, DependencySpec>();
+ Set<String> jdkPaths = new HashSet<String>();
+ JDK_MODULE_NAMES = new HashSet<String>();
// JDK
for (String module : JDKUtils.getJDKModuleNames()) {
Set<String> paths = JDKUtils.getJDKPathsByModule(module);
- DependencySpec dependencySpec = DependencySpec.createSystemDependencySpec(paths);
- SYSTEM_DEPENDENCIES.put(module, dependencySpec);
+ jdkPaths.addAll(paths);
+ JDK_MODULE_NAMES.add(module);
}
// Oracle
for (String module : JDKUtils.getOracleJDKModuleNames()) {
Set<String> paths = JDKUtils.getOracleJDKPathsByModule(module);
- DependencySpec dependencySpec = DependencySpec.createSystemDependencySpec(paths);
- SYSTEM_DEPENDENCIES.put(module, dependencySpec);
+ JDK_MODULE_NAMES.add(module);
+ jdkPaths.addAll(paths);
}
+ // always exported implicitely
+ JDK_DEPENDENCY = DependencySpec.createSystemDependencySpec(jdkPaths, true);
}
private RepositoryManager repository;
@@ -232,6 +236,12 @@ protected ModuleSpec findModule(ModuleIdentifier moduleIdentifier) throws Module
Node<ArtifactResult> root = new Node<ArtifactResult>();
for (ArtifactResult i : artifact.dependencies()) {
final String name = i.name();
+
+ // skip JDK modules
+ if(JDK_MODULE_NAMES.contains(name)) {
+ continue;
+ }
+
moduleDependencies.add(name); // track used module dependencies
if (i.importType() == ImportType.OPTIONAL) {
@@ -250,18 +260,20 @@ protected ModuleSpec findModule(ModuleIdentifier moduleIdentifier) throws Module
deps.add(mds);
}
- // no need to track system deps -- cannot be updated anyway
- if (SYSTEM_DEPENDENCIES.containsKey(name) == false) {
- ModuleIdentifier mi = createModuleIdentifier(i);
- Graph.Vertex<ModuleIdentifier, Boolean> dv = graph.createVertex(mi, mi);
- Graph.Edge.create(i.importType() == ImportType.EXPORT, vertex, dv);
- }
+ ModuleIdentifier mi = createModuleIdentifier(i);
+ Graph.Vertex<ModuleIdentifier, Boolean> dv = graph.createVertex(mi, mi);
+ Graph.Edge.create(i.importType() == ImportType.EXPORT, vertex, dv);
}
if (root.isEmpty() == false) {
LocalLoader onDemandLoader = new OnDemandLocalLoader(moduleIdentifier, this, root);
builder.setFallbackLoader(onDemandLoader);
}
}
+
+ // automagically import the JDK module
+ DependencySpec mds = JDK_DEPENDENCY;
+ builder.addDependency(mds);
+ // no need to track system deps -- cannot be updated anyway
createModuleDependency(vertex, deps, builder, LANGUAGE, false);
@@ -278,17 +290,6 @@ protected ModuleSpec findModule(ModuleIdentifier moduleIdentifier) throws Module
Graph.Vertex<ModuleIdentifier, Boolean> sdsv = graph.createVertex(RUNTIME, RUNTIME);
Graph.Edge.create(false, vertex, sdsv);
- // add JDK moduleDependencies to loose artifacts
- if (artifact.visibilityType() == VisibilityType.LOOSE) {
Aleš Justin Collaborator
alesj added a note

Does this mean that Ceylon modules now also always get JDK modules by default?

Stéphane Épardaud Owner
FroMage added a note

At runtime yes. That was the point of the issue.

Aleš Justin Collaborator
alesj added a note

But not compile time?

Stéphane Épardaud Owner
FroMage added a note

At compile-time we do check jdk modules, but we can only do so for Ceylon modules, not for Java modules. The other pb is that the JDK is not by itself modularised, so if certain "virtual" jdk modules are loaded by one classloader and others by another things won't work.

Aleš Justin Collaborator
alesj added a note

so if certain "virtual" jdk modules are loaded by one classloader and others by another things won't work.

They will all be loaded by System CL, as that's the only way Java / JVM allows it,
no matter what JBoss Modules does. ;-)

Anyway, this VisibilityType was there to prevent Ceylon Modules to drag in stuff at Runtime,
but it's your choice, so fine by me.

Stéphane Épardaud Owner
FroMage added a note

OK so same classloader, and loaded by the system classloader, unless in the case of swing it's using the context class loader, in which case some classes from one jdk module may try to load classes from another jdk module using this context class loader, which will break if we only imported a single jdk module in Ceylon. We can't be sure what the jdk will try, it's not modularised, so this change makes sure that at runtime we won't try to pretend it is, as I think it would break things. Perhaps I'm wrong, but then I need evidence to support that ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
- for (String module : JDKUtils.getJDKModuleNames()) {
- if (moduleDependencies.contains(module) == false) {
- DependencySpec ds = SYSTEM_DEPENDENCIES.get(module);
- builder.addDependency(ds);
- // no need to track system deps -- cannot be updated anyway
- }
- }
- }
-
dependencies.put(moduleIdentifier, deps);
builder.setClassFileTransformer(new UtilRegistryTransformer(moduleIdentifier, artifact));
@@ -321,9 +322,9 @@ protected void createModuleDependency(Graph.Vertex<ModuleIdentifier, Boolean> ve
* @return new module dependency
*/
DependencySpec createModuleDependency(ArtifactResult i) {
- final DependencySpec dependencySpec = SYSTEM_DEPENDENCIES.get(i.name());
- if (dependencySpec != null)
- return dependencySpec;
+ // this should never happen
+ if (JDK_MODULE_NAMES.contains(i.name()))
+ return JDK_DEPENDENCY;
final ModuleIdentifier mi = createModuleIdentifier(i);
final boolean export = (i.importType() == ImportType.EXPORT);
Aleš Justin

Does this mean that Ceylon modules now also always get JDK modules by default?

Stéphane Épardaud

At runtime yes. That was the point of the issue.

Stéphane Épardaud

At compile-time we do check jdk modules, but we can only do so for Ceylon modules, not for Java modules. The other pb is that the JDK is not by itself modularised, so if certain "virtual" jdk modules are loaded by one classloader and others by another things won't work.

Aleš Justin

so if certain "virtual" jdk modules are loaded by one classloader and others by another things won't work.

They will all be loaded by System CL, as that's the only way Java / JVM allows it,
no matter what JBoss Modules does. ;-)

Anyway, this VisibilityType was there to prevent Ceylon Modules to drag in stuff at Runtime,
but it's your choice, so fine by me.

Stéphane Épardaud

OK so same classloader, and loaded by the system classloader, unless in the case of swing it's using the context class loader, in which case some classes from one jdk module may try to load classes from another jdk module using this context class loader, which will break if we only imported a single jdk module in Ceylon. We can't be sure what the jdk will try, it's not modularised, so this change makes sure that at runtime we won't try to pretend it is, as I think it would break things. Perhaps I'm wrong, but then I need evidence to support that ;)

Please sign in to comment.
Something went wrong with that request. Please try again.