> -----Original Message-----
> From: Xavier Hanin [mailto:[EMAIL PROTECTED]
> > >
> > >          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;
> > > -        }
> >
> I see a problem with the removal of this test: if a problem occurs during
> loading or just after loading, we used to fall into this if statement, and
> we don't any more. I think we should preserver this even with your (nice)
> change adding logging to  the loading operation.
> 

I moved it before because hasProblem was already tested at the beginning of the 
function.  But you are right, there are
plenty of case where errors are actually introduced during the execution of the 
load itself.  I'm surprised that no unit
test was failing.  
Actually, the difference with the new code is that when a problem is raised 
during the load, and handlConfiguration
returns true, then we were still executing :
addDependencyArtifacts(rootModuleConf, dd.getDependencyArtifacts(parentConf));
addDependencyIncludes(rootModuleConf, dd.getIncludeRules(parentConf));

I will rollback the behaviour to what it was.

Gilles

Reply via email to