Author: gscokart
Date: Wed Nov 21 01:04:46 2007
New Revision: 596998
URL: http://svn.apache.org/viewvc?rev=596998&view=rev
Log:
IVY-644 fix NPE in case of eviction by 2 modules on 2 different confs mixed
with extends
Modified:
incubator/ivy/core/trunk/src/java/org/apache/ivy/core/resolve/IvyNode.java
incubator/ivy/core/trunk/src/java/org/apache/ivy/core/resolve/IvyNodeEviction.java
incubator/ivy/core/trunk/src/java/org/apache/ivy/core/resolve/ResolveEngine.java
incubator/ivy/core/trunk/test/java/org/apache/ivy/core/resolve/ResolveTest.java
Modified:
incubator/ivy/core/trunk/src/java/org/apache/ivy/core/resolve/IvyNode.java
URL:
http://svn.apache.org/viewvc/incubator/ivy/core/trunk/src/java/org/apache/ivy/core/resolve/IvyNode.java?rev=596998&r1=596997&r2=596998&view=diff
==============================================================================
--- incubator/ivy/core/trunk/src/java/org/apache/ivy/core/resolve/IvyNode.java
(original)
+++ incubator/ivy/core/trunk/src/java/org/apache/ivy/core/resolve/IvyNode.java
Wed Nov 21 01:04:46 2007
@@ -98,6 +98,10 @@
//CheckStyle:MagicNumber| OFF
return hash;
}
+
+ public String toString() {
+ return "NodeConf(" + conf + ")";
+ }
}
// //////// CONTEXT
@@ -182,14 +186,21 @@
*/
public boolean loadData(String rootModuleConf, IvyNode parent, String
parentConf, String conf,
boolean shouldBePublic) {
+ Message.debug("loadData of " + this.toString() + " of rootConf=" +
rootModuleConf);
if (!isRoot() && (data.getReport() != null)) {
data.getReport().addDependency(this);
}
boolean loaded = false;
- if (!isEvicted(rootModuleConf)
- && (hasConfigurationsToLoad() ||
!isRootModuleConfLoaded(rootModuleConf))
- && !hasProblem()) {
+ if (hasProblem()) {
+ Message.debug("Node has problem. Skip loading");
+ handleConfiguration(loaded, rootModuleConf, parent, parentConf,
conf, shouldBePublic);
+ return false;
+ } else if (isEvicted(rootModuleConf)) {
+ Message.debug(rootModuleConf + " is evicted. Skip loading");
+ } else if (!hasConfigurationsToLoad() &&
isRootModuleConfLoaded(rootModuleConf)) {
+ Message.debug(rootModuleConf + " is loaded and no conf to load.
Skip loading");
+ } else {
markRootModuleConfLoaded(rootModuleConf);
if (md == null) {
DependencyResolver resolver =
data.getSettings().getResolver(getModuleId());
@@ -309,11 +320,6 @@
loaded = true;
}
}
- if (hasProblem()) {
- return handleConfiguration(loaded, rootModuleConf, parent,
parentConf, conf,
- shouldBePublic)
- && loaded;
- }
if (!handleConfiguration(loaded, rootModuleConf, parent, parentConf,
conf, shouldBePublic)) {
return false;
}
@@ -347,8 +353,7 @@
"impossible to get dependencies when data has not been
loaded");
}
DependencyDescriptor[] dds = md.getDependencies();
- Collection dependencies = new LinkedHashSet(); // it's important to
respect dependencies
- // order
+ Collection dependencies = new LinkedHashSet(); // it's important to
respect order
for (int i = 0; i < dds.length; i++) {
DependencyDescriptor dd = dds[i];
String[] dependencyConfigurations =
dd.getDependencyConfigurations(conf, requestedConf);
@@ -547,6 +552,7 @@
return (String[]) depConfs.toArray(new String[depConfs.size()]);
}
+ //This is never called. Could we remove it?
public void discardConf(String rootModuleConf, String conf) {
Set depConfs = (Set) rootModuleConfs.get(rootModuleConf);
if (depConfs == null) {
@@ -750,7 +756,13 @@
// no configuration required => no artifact required
return new Artifact[0];
}
+ if (md == null) {
+ throw new IllegalStateException(
+ "impossible to get artefacts when data has not been
loaded. IvyNode = "
+ + this.toString());
+ }
+
Set artifacts = new HashSet(); // the set we fill before returning
// we check if we have dependencyArtifacts includes description for
this rootModuleConf
@@ -909,7 +921,8 @@
public ConflictManager getConflictManager(ModuleId mid) {
if (md == null) {
throw new IllegalStateException(
- "impossible to get conflict manager when data has not been
loaded");
+ "impossible to get conflict manager when data has not been
loaded. IvyNode = "
+ + this.toString());
}
ConflictManager cm = md.getConflictManager(mid);
return cm == null ? settings.getConflictManager(mid) : cm;
@@ -1097,7 +1110,9 @@
public void markEvicted(String rootModuleConf, IvyNode node,
ConflictManager conflictManager,
Collection resolved) {
- eviction.markEvicted(rootModuleConf, node, conflictManager, resolved);
+ EvictionData evictionData = new EvictionData(rootModuleConf, node,
conflictManager,
+ resolved);
+ markEvicted(evictionData);
}
public void setEvictedNodes(ModuleId moduleId, String rootModuleConf,
Collection evicted) {
Modified:
incubator/ivy/core/trunk/src/java/org/apache/ivy/core/resolve/IvyNodeEviction.java
URL:
http://svn.apache.org/viewvc/incubator/ivy/core/trunk/src/java/org/apache/ivy/core/resolve/IvyNodeEviction.java?rev=596998&r1=596997&r2=596998&view=diff
==============================================================================
---
incubator/ivy/core/trunk/src/java/org/apache/ivy/core/resolve/IvyNodeEviction.java
(original)
+++
incubator/ivy/core/trunk/src/java/org/apache/ivy/core/resolve/IvyNodeEviction.java
Wed Nov 21 01:04:46 2007
@@ -91,6 +91,10 @@
public String getRootModuleConf() {
return rootModuleConf;
}
+
+ public boolean isTransitivelyEvicted() {
+ return parent == null;
+ }
}
private static final class ModuleIdConf {
@@ -235,9 +239,13 @@
public boolean isEvicted(String rootModuleConf) {
cleanEvicted();
IvyNode root = node.getRoot();
- return root != node
- && !root.getResolvedRevisions(node.getId().getModuleId(),
rootModuleConf).contains(
- node.getResolvedId()) && getEvictedData(rootModuleConf) !=
null;
+ ModuleId moduleId = node.getId().getModuleId();
+ Collection resolvedRevisions = root.getResolvedRevisions(moduleId,
rootModuleConf);
+ EvictionData evictedData = getEvictedData(rootModuleConf);
+ return root != node && evictedData != null
+ && (!resolvedRevisions.contains(node.getResolvedId())
+ || evictedData.isTransitivelyEvicted()
+ );
}
public boolean isCompletelyEvicted() {
@@ -271,13 +279,6 @@
}
}
}
- }
-
- public void markEvicted(String rootModuleConf, IvyNode node,
ConflictManager conflictManager,
- Collection resolved) {
- EvictionData evictionData = new EvictionData(rootModuleConf, node,
conflictManager,
- resolved);
- markEvicted(evictionData);
}
public void markEvicted(EvictionData evictionData) {
Modified:
incubator/ivy/core/trunk/src/java/org/apache/ivy/core/resolve/ResolveEngine.java
URL:
http://svn.apache.org/viewvc/incubator/ivy/core/trunk/src/java/org/apache/ivy/core/resolve/ResolveEngine.java?rev=596998&r1=596997&r2=596998&view=diff
==============================================================================
---
incubator/ivy/core/trunk/src/java/org/apache/ivy/core/resolve/ResolveEngine.java
(original)
+++
incubator/ivy/core/trunk/src/java/org/apache/ivy/core/resolve/ResolveEngine.java
Wed Nov 21 01:04:46 2007
@@ -548,10 +548,11 @@
} else {
Message.verbose("== resolving dependencies for " + node.getId() +
" [" + conf + "]");
}
- resolveConflict(node);
+ resolveConflict(node, conf);
if (node.loadData(conf, shouldBePublic)) {
- resolveConflict(node);
+ resolveConflict(node, conf); //We just did it. Should it be
redone?
+ //NB:removing it break a unit test.
if (!node.isEvicted()) {
String[] confs = node.getRealConfs(conf);
for (int i = 0; i < confs.length; i++) {
@@ -672,8 +673,8 @@
return false;
}
- private void resolveConflict(VisitNode node) {
- resolveConflict(node, node.getParent(), Collections.EMPTY_SET);
+ private void resolveConflict(VisitNode node, String conf) {
+ resolveConflict(node, node.getParent(), conf,Collections.EMPTY_SET);
}
/**
@@ -689,7 +690,8 @@
* descendants of ancestor)
* @return true if conflict resolution has been done, false it can't be
done yet
*/
- private boolean resolveConflict(VisitNode node, VisitNode ancestor,
Collection toevict) {
+ private boolean resolveConflict(VisitNode node, VisitNode ancestor, String
conf,
+ Collection toevict) {
if (ancestor == null || node == ancestor) {
return true;
}
@@ -702,7 +704,7 @@
// job is done and node is selected, nothing to do for this
ancestor, but we still have
// to check higher levels, for which conflict resolution might
have been impossible
// before
- if (resolveConflict(node, ancestor.getParent(), toevict)) {
+ if (resolveConflict(node, ancestor.getParent(), conf, toevict)) {
// now that conflict resolution is ok in ancestors
// we just have to check if the node wasn't previously evicted
in root ancestor
EvictionData evictionData =
node.getEvictionDataInRoot(node.getRootModuleConf(),
@@ -730,7 +732,7 @@
node.getModuleId(), node.getRootModuleConf()));
resolvedNodes.addAll(ancestor.getNode().getPendingConflicts(node.getRootModuleConf(),
node.getModuleId()));
- Collection conflicts = computeConflicts(node, ancestor, toevict,
resolvedNodes);
+ Collection conflicts = computeConflicts(node, ancestor, conf, toevict,
resolvedNodes);
if (settings.debugConflictResolution()) {
Message.debug("found conflicting revisions for " + node + " in " +
ancestor + ": "
@@ -788,7 +790,7 @@
ancestor.getNode().setPendingConflicts(node.getModuleId(),
node.getRootModuleConf(),
Collections.EMPTY_SET);
- return resolveConflict(node, ancestor.getParent(), toevict);
+ return resolveConflict(node, ancestor.getParent(), conf, toevict);
} else {
// node has been evicted for the current parent
if (resolved.isEmpty()) {
@@ -827,7 +829,7 @@
IvyNode sel = (IvyNode) iter.next();
if (!prevResolved.contains(sel)) {
solved &= resolveConflict(node.gotoNode(sel),
ancestor.getParent(),
- toevict);
+ conf, toevict);
}
}
}
@@ -835,8 +837,8 @@
}
}
- private Collection computeConflicts(VisitNode node, VisitNode ancestor,
Collection toevict,
- Collection resolvedNodes) {
+ private Collection computeConflicts(VisitNode node, VisitNode ancestor,
String conf,
+ Collection toevict, Collection resolvedNodes) {
Collection conflicts = new HashSet();
conflicts.add(node.getNode());
if (resolvedNodes.removeAll(toevict)) {
@@ -853,14 +855,13 @@
.addAll(dep.getResolvedNodes(node.getModuleId(),
node.getRootModuleConf()));
}
} else if (resolvedNodes.isEmpty() && node.getParent() != ancestor) {
- DependencyDescriptor[] dds =
ancestor.getDescriptor().getDependencies();
- for (int i = 0; i < dds.length; i++) {
- if (dds[i].getDependencyId().equals(node.getModuleId())) {
- IvyNode n =
node.getNode().findNode(dds[i].getDependencyRevisionId());
- if (n != null) {
- conflicts.add(n);
- break;
- }
+ //Conflict must only be computed per root configuration at this
step.
+ Collection ancestorDepIvyNodes = node.getParent().getNode()
+ .getDependencies(node.getRootModuleConf(), new
String[] {conf});
+ for (Iterator it = ancestorDepIvyNodes.iterator(); it.hasNext();) {
+ IvyNode ancestorDep = (IvyNode) it.next();
+ if (ancestorDep.getModuleId().equals(node.getModuleId())) {
+ conflicts.add(ancestorDep);
}
}
} else {
Modified:
incubator/ivy/core/trunk/test/java/org/apache/ivy/core/resolve/ResolveTest.java
URL:
http://svn.apache.org/viewvc/incubator/ivy/core/trunk/test/java/org/apache/ivy/core/resolve/ResolveTest.java?rev=596998&r1=596997&r2=596998&view=diff
==============================================================================
---
incubator/ivy/core/trunk/test/java/org/apache/ivy/core/resolve/ResolveTest.java
(original)
+++
incubator/ivy/core/trunk/test/java/org/apache/ivy/core/resolve/ResolveTest.java
Wed Nov 21 01:04:46 2007
@@ -18,9 +18,6 @@
package org.apache.ivy.core.resolve;
import java.io.File;
-import java.io.IOException;
-import java.net.MalformedURLException;
-import java.text.ParseException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
@@ -35,7 +32,6 @@
import org.apache.ivy.Ivy;
import org.apache.ivy.TestHelper;
-import org.apache.ivy.core.IvyContext;
import org.apache.ivy.core.cache.ArtifactOrigin;
import org.apache.ivy.core.cache.CacheManager;
import org.apache.ivy.core.module.descriptor.Artifact;
@@ -58,9 +54,7 @@
import org.apache.ivy.plugins.resolver.DualResolver;
import org.apache.ivy.plugins.resolver.FileSystemResolver;
import org.apache.ivy.util.CacheCleaner;
-import org.apache.ivy.util.DefaultMessageLogger;
import org.apache.ivy.util.FileUtil;
-import org.apache.ivy.util.Message;
import org.apache.tools.ant.Project;
import org.apache.tools.ant.taskdefs.Delete;
import org.xml.sax.SAXException;
@@ -675,8 +669,6 @@
ModuleRevisionId.newInstance("org1", "mod1.2", "2.1")).exists());
assertTrue(getArchiveFileInCache("org1", "mod1.2", "2.1", "mod1.2",
"jar", "jar").exists());
- assertFalse(cacheManager.getIvyFileInCache(
- ModuleRevisionId.newInstance("org1", "mod1.2", "2.0")).exists());
assertFalse(getArchiveFileInCache("org1", "mod1.2", "2.0", "mod1.2",
"jar", "jar").exists());
}
@@ -1474,7 +1466,7 @@
public void testTransitiveEviction() throws Exception {
// mod7.3 depends on mod7.2 v1.0 and on mod7.1 v2.0
// mod7.2 v1.0 depends on mod7.1 v1.0 (which then should be evicted)
- // mod7.1 v1.0 depends on mod 1.2 v1.0 (which should be evicted by
transitivity)
+ // mod7.1 v1.0 depends on mod 1.2 v2.0 (which should be evicted by
transitivity)
ResolveReport report = ivy.resolve(new
File("test/repositories/2/mod7.3/ivy-1.0.xml")
.toURL(), getResolveOptions(new String[] {"*"}));
@@ -1495,9 +1487,9 @@
ModuleRevisionId.newInstance("org7", "mod7.1", "2.0")).exists());
assertTrue(getArchiveFileInCache("org7", "mod7.1", "2.0", "mod7.1",
"jar", "jar").exists());
- assertTrue(!getArchiveFileInCache("org7", "mod7.1", "1.0", "mod7.1",
"jar", "jar").exists());
+ assertFalse(getArchiveFileInCache("org7", "mod7.1", "1.0", "mod7.1",
"jar", "jar").exists());
- assertTrue(!getArchiveFileInCache("org1", "mod1.2", "2.0", "mod1.2",
"jar", "jar").exists());
+ assertFalse(getArchiveFileInCache("org1", "mod1.2", "2.0", "mod1.2",
"jar", "jar").exists());
}
public void testTransitiveEviction2() throws Exception {
@@ -1753,15 +1745,15 @@
.newInstance("org5", "mod5.1", "4.2")).length);
}
- /*
+
public void testMultipleEviction() throws Exception {
ResolveReport report = ivy.resolve(
new
File("test/repositories/1/IVY-644/M1/ivys/ivy-1.0.xml").toURL(),
- getResolveOptions(new String[] {"*"}));
+ getResolveOptions(new String[] {"test" , "runtime" })); //NB the
order impact the bug
assertFalse(report.hasError());
}
- */
+
public void testResolveForce() throws Exception {
// mod4.1 v 4.2 depends on
