From 74f13972f7abb04ad0a7e1540d7c663d8c81819c Mon Sep 17 00:00:00 2001 From: Eugene Zhuravlev Date: Thu, 20 Feb 2014 19:31:48 +0400 Subject: [PATCH] fixing compiler storage data integrity for certain cases when classes are moved between modules --- ...moveClassFromJavaFileToDependentModule.log | 10 +++ .../moduleA/src/com/ppp/Inner.java.new | 4 + .../moduleB/src/com/ppp/B.java | 8 ++ .../moduleB/src/com/ppp/B.java.new | 5 ++ .../common/moveClassToDependentModule.log | 6 ++ .../moduleA/src/com/ppp/Inner.java.new | 4 + .../moduleB/src/com/ppp/B.java | 5 ++ .../moduleB/src/com/ppp/Inner.java | 5 ++ .../moduleB/src/com/ppp/Inner.java.remove | 0 .../java/dependencyView/Mappings.java | 90 +++++++++---------- .../org/jetbrains/ether/CommonTest.java | 17 ++++ 11 files changed, 109 insertions(+), 45 deletions(-) create mode 100644 java/java-tests/testData/compileServer/incremental/common/moveClassFromJavaFileToDependentModule.log create mode 100644 java/java-tests/testData/compileServer/incremental/common/moveClassFromJavaFileToDependentModule/moduleA/src/com/ppp/Inner.java.new create mode 100644 java/java-tests/testData/compileServer/incremental/common/moveClassFromJavaFileToDependentModule/moduleB/src/com/ppp/B.java create mode 100644 java/java-tests/testData/compileServer/incremental/common/moveClassFromJavaFileToDependentModule/moduleB/src/com/ppp/B.java.new create mode 100644 java/java-tests/testData/compileServer/incremental/common/moveClassToDependentModule.log create mode 100644 java/java-tests/testData/compileServer/incremental/common/moveClassToDependentModule/moduleA/src/com/ppp/Inner.java.new create mode 100644 java/java-tests/testData/compileServer/incremental/common/moveClassToDependentModule/moduleB/src/com/ppp/B.java create mode 100644 java/java-tests/testData/compileServer/incremental/common/moveClassToDependentModule/moduleB/src/com/ppp/Inner.java create mode 100644 java/java-tests/testData/compileServer/incremental/common/moveClassToDependentModule/moduleB/src/com/ppp/Inner.java.remove diff --git a/java/java-tests/testData/compileServer/incremental/common/moveClassFromJavaFileToDependentModule.log b/java/java-tests/testData/compileServer/incremental/common/moveClassFromJavaFileToDependentModule.log new file mode 100644 index 000000000000..125f8bf37d22 --- /dev/null +++ b/java/java-tests/testData/compileServer/incremental/common/moveClassFromJavaFileToDependentModule.log @@ -0,0 +1,10 @@ +Compiling files: +moduleA/src/com/ppp/Inner.java +End of files +Cleaning output files: +out/production/moduleB/com/ppp/B.class +out/production/moduleB/com/ppp/Inner.class +End of files +Compiling files: +moduleB/src/com/ppp/B.java +End of files diff --git a/java/java-tests/testData/compileServer/incremental/common/moveClassFromJavaFileToDependentModule/moduleA/src/com/ppp/Inner.java.new b/java/java-tests/testData/compileServer/incremental/common/moveClassFromJavaFileToDependentModule/moduleA/src/com/ppp/Inner.java.new new file mode 100644 index 000000000000..b27489774bed --- /dev/null +++ b/java/java-tests/testData/compileServer/incremental/common/moveClassFromJavaFileToDependentModule/moduleA/src/com/ppp/Inner.java.new @@ -0,0 +1,4 @@ +package com.ppp; + +class Inner { +} \ No newline at end of file diff --git a/java/java-tests/testData/compileServer/incremental/common/moveClassFromJavaFileToDependentModule/moduleB/src/com/ppp/B.java b/java/java-tests/testData/compileServer/incremental/common/moveClassFromJavaFileToDependentModule/moduleB/src/com/ppp/B.java new file mode 100644 index 000000000000..d49fc1b78e96 --- /dev/null +++ b/java/java-tests/testData/compileServer/incremental/common/moveClassFromJavaFileToDependentModule/moduleB/src/com/ppp/B.java @@ -0,0 +1,8 @@ +package com.ppp; + +public class B { +} + +class Inner { +} + diff --git a/java/java-tests/testData/compileServer/incremental/common/moveClassFromJavaFileToDependentModule/moduleB/src/com/ppp/B.java.new b/java/java-tests/testData/compileServer/incremental/common/moveClassFromJavaFileToDependentModule/moduleB/src/com/ppp/B.java.new new file mode 100644 index 000000000000..71be7afd34a8 --- /dev/null +++ b/java/java-tests/testData/compileServer/incremental/common/moveClassFromJavaFileToDependentModule/moduleB/src/com/ppp/B.java.new @@ -0,0 +1,5 @@ +package com.ppp; + +public class B { +} + diff --git a/java/java-tests/testData/compileServer/incremental/common/moveClassToDependentModule.log b/java/java-tests/testData/compileServer/incremental/common/moveClassToDependentModule.log new file mode 100644 index 000000000000..338a0874d33c --- /dev/null +++ b/java/java-tests/testData/compileServer/incremental/common/moveClassToDependentModule.log @@ -0,0 +1,6 @@ +Compiling files: +moduleA/src/com/ppp/Inner.java +End of files +Cleaning output files: +out/production/moduleB/com/ppp/Inner.class +End of files diff --git a/java/java-tests/testData/compileServer/incremental/common/moveClassToDependentModule/moduleA/src/com/ppp/Inner.java.new b/java/java-tests/testData/compileServer/incremental/common/moveClassToDependentModule/moduleA/src/com/ppp/Inner.java.new new file mode 100644 index 000000000000..b27489774bed --- /dev/null +++ b/java/java-tests/testData/compileServer/incremental/common/moveClassToDependentModule/moduleA/src/com/ppp/Inner.java.new @@ -0,0 +1,4 @@ +package com.ppp; + +class Inner { +} \ No newline at end of file diff --git a/java/java-tests/testData/compileServer/incremental/common/moveClassToDependentModule/moduleB/src/com/ppp/B.java b/java/java-tests/testData/compileServer/incremental/common/moveClassToDependentModule/moduleB/src/com/ppp/B.java new file mode 100644 index 000000000000..71be7afd34a8 --- /dev/null +++ b/java/java-tests/testData/compileServer/incremental/common/moveClassToDependentModule/moduleB/src/com/ppp/B.java @@ -0,0 +1,5 @@ +package com.ppp; + +public class B { +} + diff --git a/java/java-tests/testData/compileServer/incremental/common/moveClassToDependentModule/moduleB/src/com/ppp/Inner.java b/java/java-tests/testData/compileServer/incremental/common/moveClassToDependentModule/moduleB/src/com/ppp/Inner.java new file mode 100644 index 000000000000..5ffa5766c98e --- /dev/null +++ b/java/java-tests/testData/compileServer/incremental/common/moveClassToDependentModule/moduleB/src/com/ppp/Inner.java @@ -0,0 +1,5 @@ +package com.ppp; + +class Inner { +} + diff --git a/java/java-tests/testData/compileServer/incremental/common/moveClassToDependentModule/moduleB/src/com/ppp/Inner.java.remove b/java/java-tests/testData/compileServer/incremental/common/moveClassToDependentModule/moduleB/src/com/ppp/Inner.java.remove new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/Mappings.java b/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/Mappings.java index c4121fc2ffba..bb32c8b172e3 100644 --- a/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/Mappings.java +++ b/jps/jps-builders/src/org/jetbrains/jps/builders/java/dependencyView/Mappings.java @@ -66,7 +66,7 @@ public class Mappings { private final TIntHashSet myChangedClasses; private final THashSet myChangedFiles; - private final Set myDeletedClasses; + private final Set> myDeletedClasses; private final Set myAddedClasses; private final Object myLock; private final File myRootDir; @@ -102,7 +102,7 @@ public class Mappings { myIsDelta = true; myChangedClasses = new TIntHashSet(DEFAULT_SET_CAPACITY, DEFAULT_SET_LOAD_FACTOR); myChangedFiles = new THashSet(FileUtil.FILE_HASHING_STRATEGY); - myDeletedClasses = new HashSet(DEFAULT_SET_CAPACITY, DEFAULT_SET_LOAD_FACTOR); + myDeletedClasses = new HashSet>(DEFAULT_SET_CAPACITY, DEFAULT_SET_LOAD_FACTOR); myAddedClasses = new HashSet(DEFAULT_SET_CAPACITY, DEFAULT_SET_LOAD_FACTOR); myDeltaIsTransient = base.myDeltaIsTransient; myRootDir = new File(FileUtil.toSystemIndependentName(base.myRootDir.getAbsolutePath()) + File.separatorChar + "myDelta"); @@ -222,10 +222,10 @@ public class Mappings { private final LinkedBlockingQueue myPostPasses = new LinkedBlockingQueue(); private void runPostPasses() { - final Set deleted = myDeletedClasses; + final Set> deleted = myDeletedClasses; if (deleted != null) { - for (ClassRepr repr : deleted) { - myChangedClasses.remove(repr.name); + for (Pair pair : deleted) { + myChangedClasses.remove(pair.first.name); } } for (Runnable pass = myPostPasses.poll(); pass != null; pass = myPostPasses.poll()) { @@ -687,24 +687,21 @@ public class Mappings { } } - void affectAll(final int className, final Collection affectedFiles, @Nullable final DependentFilesFilter filter) { - final File sourceFile = myClassToSourceFile.get(className); - if (sourceFile != null) { - final TIntHashSet dependants = myClassToClassDependency.get(className); - if (dependants != null) { - dependants.forEach(new TIntProcedure() { - @Override - public boolean execute(int depClass) { - final File depFile = myClassToSourceFile.get(depClass); - if (depFile != null && !FileUtil.filesEqual(depFile, sourceFile)) { - if (filter == null || filter.accept(depFile)) { - affectedFiles.add(depFile); - } + void affectAll(final int className, @NotNull final File sourceFile, final Collection affectedFiles, @Nullable final DependentFilesFilter filter) { + final TIntHashSet dependants = myClassToClassDependency.get(className); + if (dependants != null) { + dependants.forEach(new TIntProcedure() { + @Override + public boolean execute(int depClass) { + final File depFile = myClassToSourceFile.get(depClass); + if (depFile != null && !FileUtil.filesEqual(depFile, sourceFile)) { + if (filter == null || filter.accept(depFile)) { + affectedFiles.add(depFile); } - return true; } - }); - } + return true; + } + }); } } @@ -1003,12 +1000,13 @@ public class Mappings { if (removed != null) { for (final String file : removed) { - final Collection classes = mySourceFileToClasses.get(new File(file)); + final File sourceFile = new File(file); + final Collection classes = mySourceFileToClasses.get(sourceFile); if (classes != null) { for (ClassRepr c : classes) { debug("Affecting usages of removed class ", c.name); - affectAll(c.name, myAffectedFiles, myFilter); + affectAll(c.name, sourceFile, myAffectedFiles, myFilter); } } } @@ -1743,27 +1741,23 @@ public class Mappings { return !myEasyMode; } - private void processRemovedClases(final DiffState state) { + private void processRemovedClases(final DiffState state, @NotNull File fileName) { final Collection removed = state.myClassDiff.removed(); if (removed.isEmpty()) { return; } + myDelta.myChangedFiles.add(fileName); + debug("Processing removed classes:"); + for (final ClassRepr c : removed) { - myDelta.addDeletedClass(c); - - final File fileName = myClassToSourceFile.get(c.name); - - if (fileName != null) { - myDelta.myChangedFiles.add(fileName); - } - + myDelta.addDeletedClass(c, fileName); if (!myEasyMode) { myPresent.appendDependents(c, state.myDependants); debug("Adding usages of class ", c.name); state.myAffectedUsages.add(c.createUsage()); debug("Affecting usages of removed class ", c.name); - affectAll(c.name, myAffectedFiles, myFilter); + affectAll(c.name, fileName, myAffectedFiles, myFilter); } } debug("End of removed classes processing."); @@ -1939,7 +1933,7 @@ public class Mappings { } } - processRemovedClases(state); + processRemovedClases(state, fileName); processAddedClasses(state, fileName); if (!myEasyMode) { @@ -2003,8 +1997,13 @@ public class Mappings { } } - private void cleanupRemovedClass(final Mappings delta, @NotNull final ClassRepr cr, final Set usages, final IntIntMultiMaplet dependenciesTrashBin) { + private void cleanupRemovedClass(final Mappings delta, @NotNull final ClassRepr cr, File sourceFile, final Set usages, final IntIntMultiMaplet dependenciesTrashBin) { final int className = cr.name; + if (!FileUtil.filesEqual(sourceFile, myClassToSourceFile.get(className))) { + // if classname is already mapped to a different source, the class with such FQ name exists elsewhere, so + // we cannot destroy all these links + return; + } for (final int superSomething : cr.getSupers()) { delta.registerRemovedSuperClass(className, superSomething); @@ -2033,21 +2032,22 @@ public class Mappings { if (removed != null) { for (final String file : removed) { - final File fileName = new File(file); - final Set fileClasses = (Set)mySourceFileToClasses.get(fileName); + final File deletedFile = new File(file); + final Set fileClasses = (Set)mySourceFileToClasses.get(deletedFile); if (fileClasses != null) { for (final ClassRepr aClass : fileClasses) { - cleanupRemovedClass(delta, aClass, aClass.getUsages(), dependenciesTrashBin); + cleanupRemovedClass(delta, aClass, deletedFile, aClass.getUsages(), dependenciesTrashBin); } - mySourceFileToClasses.remove(fileName); + mySourceFileToClasses.remove(deletedFile); } } } if (!delta.isRebuild()) { - for (final ClassRepr repr : delta.getDeletedClasses()) { - cleanupRemovedClass(delta, repr, repr.getUsages(), dependenciesTrashBin); + for (final Pair pair : delta.getDeletedClasses()) { + final ClassRepr deletedClass = pair.first; + cleanupRemovedClass(delta, deletedClass, pair.second, deletedClass.getUsages(), dependenciesTrashBin); } for (ClassRepr repr : delta.getAddedClasses()) { if (!repr.isAnonymous() && !repr.isLocal()) { @@ -2345,10 +2345,10 @@ public class Mappings { return myIsRebuild; } - private void addDeletedClass(final ClassRepr cr) { + private void addDeletedClass(final ClassRepr cr, File fileName) { assert (myDeletedClasses != null); - myDeletedClasses.add(cr); + myDeletedClasses.add(Pair.create(cr, fileName)); addChangedClass(cr.name); } @@ -2373,8 +2373,8 @@ public class Mappings { } @NotNull - private Set getDeletedClasses() { - return myDeletedClasses == null ? Collections.emptySet() : Collections.unmodifiableSet(myDeletedClasses); + private Set> getDeletedClasses() { + return myDeletedClasses == null ? Collections.>emptySet() : Collections.unmodifiableSet(myDeletedClasses); } @NotNull diff --git a/jps/jps-builders/testSrc/org/jetbrains/ether/CommonTest.java b/jps/jps-builders/testSrc/org/jetbrains/ether/CommonTest.java index aeb6fdeeecb4..ee72b356dc62 100644 --- a/jps/jps-builders/testSrc/org/jetbrains/ether/CommonTest.java +++ b/jps/jps-builders/testSrc/org/jetbrains/ether/CommonTest.java @@ -15,6 +15,9 @@ */ package org.jetbrains.ether; +import org.jetbrains.jps.model.JpsModuleRootModificationUtil; +import org.jetbrains.jps.model.module.JpsModule; + /** * @author: db * Date: 22.09.11 @@ -122,4 +125,18 @@ public class CommonTest extends IncrementalTestCase { doTest(); } + public void testMoveClassToDependentModule() throws Exception { + JpsModule moduleA = addModule("moduleA", "moduleA/src"); + JpsModule moduleB = addModule("moduleB", "moduleB/src"); + JpsModuleRootModificationUtil.addDependency(moduleB, moduleA); + doTestBuild(1).assertSuccessful(); + } + + public void testMoveClassFromJavaFileToDependentModule() throws Exception { + JpsModule moduleA = addModule("moduleA", "moduleA/src"); + JpsModule moduleB = addModule("moduleB", "moduleB/src"); + JpsModuleRootModificationUtil.addDependency(moduleB, moduleA); + doTestBuild(1).assertSuccessful(); + } + }