Derby
  1. Derby
  2. DERBY-4845

Improve the dependency tracking for our build targets

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 10.8.1.2
    • Fix Version/s: None
    • Component/s: Build tools
    • Urgency:
      Normal

      Description

      Derby is supposed to have an incremental build. That is, if you change a file, then the following command should recompile the file:

      ant all

      There are several places in the graph of Derby build targets where this is not true. If you change a file in certain packages, a subsequent "ant all" won't notice the change. This issue is a place where we can track these bugs.

      1. d4845-jsr169-nonoptional.diff
        9 kB
        Knut Anders Hatlen
      2. d4845-iapi-exception-util.diff
        24 kB
        Knut Anders Hatlen
      3. d4845-iapi-brokered-callable-stmt.diff
        6 kB
        Knut Anders Hatlen
      4. d4845-iapi-brokered-connection.diff
        7 kB
        Knut Anders Hatlen
      5. d4845-iapi-stmt-isClosed.diff
        6 kB
        Knut Anders Hatlen
      6. d4845-buildbreak.diff
        13 kB
        Knut Anders Hatlen
      7. d4845-iapi-readOnlyUpgrade.diff
        6 kB
        Knut Anders Hatlen
      8. d4845-iapi-unused-imports.diff
        3 kB
        Knut Anders Hatlen
      9. d4845-iapi-nodefactory-getnode.diff
        23 kB
        Knut Anders Hatlen
      10. d4845-iapi-slimmer-nodefactory.diff
        19 kB
        Knut Anders Hatlen
      11. d4845-iapi-dependablefinder.diff
        11 kB
        Knut Anders Hatlen
      12. d4845-iapi-resultsetstatistics.diff
        24 kB
        Knut Anders Hatlen
      13. d4845-iapi-statementnode.diff
        14 kB
        Knut Anders Hatlen
      14. derby-4845-02-aa-flipUtilsAndTypes.diff
        0.7 kB
        Rick Hillegas
      15. derby-4845-01-aa-removeParserPrep
        0.9 kB
        Rick Hillegas
      16. tools-i18n.diff
        1 kB
        Knut Anders Hatlen
      17. check-build.ksh
        0.6 kB
        Knut Anders Hatlen

        Issue Links

          Activity

          Hide
          Dag H. Wanvik added a comment -

          I have noticed that this is the case for package iapi/util/*.java. There may be others.

          Show
          Dag H. Wanvik added a comment - I have noticed that this is the case for package iapi/util/*.java. There may be others.
          Hide
          Dag H. Wanvik added a comment -

          Committed a small fix to the build file for package iapi/util/ which makes "ant all" pick up changes in files: svn1023167

          Show
          Dag H. Wanvik added a comment - Committed a small fix to the build file for package iapi/util/ which makes "ant all" pick up changes in files: svn1023167
          Hide
          Knut Anders Hatlen added a comment -

          I used the attached script to find out which files were not picked up by an incremental build. Here's what I found:

          • java/tools/org/apache/derby/iapi/tools/i18n/LocalizedResource.java

          Modifications to this file are detected by incremental builds, but it will be compiled using another build target than what's used by full builds. Full builds will compile against Java 1.4.2 libraries, whereas incremental builds will compile against the JSR-169 stubs. Incremental builds fail because the class calls ResultSet.getBigDecimal(int), which is not part of JSR-169.

          • java/engine/org/apache/derby/jdbc/EmbedXAResource.java
          • java/engine/org/apache/derby/jdbc/JDBC.java
          • java/engine/org/apache/derby/iapi/transaction/TransactionControl.java
          • java/engine/org/apache/derby/iapi/transaction/TransactionListener.java
          • java/engine/org/apache/derby/iapi/security/SecurityUtil.java
          • java/drda/org/apache/derby/mbeans/drda/NetworkServerMBean.java

          Modifications to these files are not detected by incremental builds.

          In addition, none of the files under java/demo/localcal and java/demo/scores are picked up by incremental builds, but they are not compiled by full builds either.

          Show
          Knut Anders Hatlen added a comment - I used the attached script to find out which files were not picked up by an incremental build. Here's what I found: java/tools/org/apache/derby/iapi/tools/i18n/LocalizedResource.java Modifications to this file are detected by incremental builds, but it will be compiled using another build target than what's used by full builds. Full builds will compile against Java 1.4.2 libraries, whereas incremental builds will compile against the JSR-169 stubs. Incremental builds fail because the class calls ResultSet.getBigDecimal(int), which is not part of JSR-169. java/engine/org/apache/derby/jdbc/EmbedXAResource.java java/engine/org/apache/derby/jdbc/JDBC.java java/engine/org/apache/derby/iapi/transaction/TransactionControl.java java/engine/org/apache/derby/iapi/transaction/TransactionListener.java java/engine/org/apache/derby/iapi/security/SecurityUtil.java java/drda/org/apache/derby/mbeans/drda/NetworkServerMBean.java Modifications to these files are not detected by incremental builds. In addition, none of the files under java/demo/localcal and java/demo/scores are picked up by incremental builds, but they are not compiled by full builds either.
          Hide
          Knut Anders Hatlen added a comment -

          Here's a patch for the issue with tools/i18n/LocalizedResource.java.

          M java/tools/build.xml

          The classes in impl/tools depend on the classes in iapi/tools, but there are no dependencies the other way around. Therefore, turn around the order of the targets and build iapi before impl. This change ensures that LocalizedResource.java is always compiled against the JSR-169 libraries, so it will now fail to compile both in full builds and in incremental builds.

          M java/tools/org/apache/derby/iapi/tools/i18n/LocalizedResource.java

          Call getObject() instead of getBigDecimal() when retrieving the value of a DECIMAL column. Since getObject() is part of JSR-169, this will make the class compilable again. getObject() returns a java.math.BigDecimal, so there should be no change for the embedded driver and the client driver. The JSR-169 driver throws an exception when getObject() is called on a DECIMAL column, but LocalizedResource already does introspection to find out if it's safe to retrieve BigDecimal values. On JSR-169 it'll fall back to getString() as it did before.

          Show
          Knut Anders Hatlen added a comment - Here's a patch for the issue with tools/i18n/LocalizedResource.java. M java/tools/build.xml The classes in impl/tools depend on the classes in iapi/tools, but there are no dependencies the other way around. Therefore, turn around the order of the targets and build iapi before impl. This change ensures that LocalizedResource.java is always compiled against the JSR-169 libraries, so it will now fail to compile both in full builds and in incremental builds. M java/tools/org/apache/derby/iapi/tools/i18n/LocalizedResource.java Call getObject() instead of getBigDecimal() when retrieving the value of a DECIMAL column. Since getObject() is part of JSR-169, this will make the class compilable again. getObject() returns a java.math.BigDecimal, so there should be no change for the embedded driver and the client driver. The JSR-169 driver throws an exception when getObject() is called on a DECIMAL column, but LocalizedResource already does introspection to find out if it's safe to retrieve BigDecimal values. On JSR-169 it'll fall back to getString() as it did before.
          Hide
          Knut Anders Hatlen added a comment -

          Tests ran cleanly with tools-i18n.diff. Committed revision 1023662.

          Show
          Knut Anders Hatlen added a comment - Tests ran cleanly with tools-i18n.diff. Committed revision 1023662.
          Hide
          Dag H. Wanvik added a comment -

          Thanks, Knut! Another build anomaly I noticed:
          a) ant clobber
          b) introduce a syntax error to a file in org.apache.derby.iapi.util, e.g. ByteArray.java
          c) ant all, build stops
          d) correct error
          e) ant all now fails with this error:
          C:\cygwin\home\dag\java\sb\sb1\generated\java\org\apache\derby\impl\sql\compile\SQLParser.java:25: package org.apache.derby.iapi.sql does not exist
          [javac] import org.apache.derby.iapi.sql.Statement;

          f) ant clobber all makes all ok again

          Show
          Dag H. Wanvik added a comment - Thanks, Knut! Another build anomaly I noticed: a) ant clobber b) introduce a syntax error to a file in org.apache.derby.iapi.util, e.g. ByteArray.java c) ant all, build stops d) correct error e) ant all now fails with this error: C:\cygwin\home\dag\java\sb\sb1\generated\java\org\apache\derby\impl\sql\compile\SQLParser.java:25: package org.apache.derby.iapi.sql does not exist [javac] import org.apache.derby.iapi.sql.Statement; f) ant clobber all makes all ok again
          Hide
          Rick Hillegas added a comment -

          Attaching derby-4845-01-aa-removeParserPrep.diff. This patch cleans up an asymmetry in the generation of Derby's grammars. Committed at subversion revision 1031069.

          This patch should fix certain situations in which you see the following:

          1) You build the codeline from scratch.

          2) But the build dies on a compile-time error.

          3) You fix the compile-time error and try "ant all" without doing a clobber, hoping for an incremental finish to the interrupted build.

          4) But now the build dies on unsatisfied references in the generated grammars.

          Touches the following files:

          -------------

          M java/tools/org/apache/derby/impl/tools/build.xml

          Make the tools build depend on generation of the ij grammar the same way that the sql interpreter build depends on generation of the sql grammar.

          -------------

          M build.xml

          Stop generating the grammars before they are really needed.

          Show
          Rick Hillegas added a comment - Attaching derby-4845-01-aa-removeParserPrep.diff. This patch cleans up an asymmetry in the generation of Derby's grammars. Committed at subversion revision 1031069. This patch should fix certain situations in which you see the following: 1) You build the codeline from scratch. 2) But the build dies on a compile-time error. 3) You fix the compile-time error and try "ant all" without doing a clobber, hoping for an incremental finish to the interrupted build. 4) But now the build dies on unsatisfied references in the generated grammars. Touches the following files: ------------- M java/tools/org/apache/derby/impl/tools/build.xml Make the tools build depend on generation of the ij grammar the same way that the sql interpreter build depends on generation of the sql grammar. ------------- M build.xml Stop generating the grammars before they are really needed.
          Hide
          Rick Hillegas added a comment -

          Attaching derby-4845-02-aa-flipUtilsAndTypes.diff. This patch fixes a build dependency problem which surfaced when I sync'd with the head of the trunk. Committed at subversion revision 1031174.

          I think that the problem was caused by some submission which made the compilation of ...iapi.util start compiling what's in iapi.types. That latter directory has a special build target which compiles SqlXmlUtil with a special classpath which includes xercesImpl.jar. You can't just compile the contents of iapi.types with the ordinary jdk1.4 classpath.

          The solution was to flip the order in which we compile ...iapi.util and ...iapi.types. Now we compile ...iapi.types first.

          Hopefully this patch doesn't destabilize someone else's build.

          Touches the following file:

          M java/engine/org/apache/derby/iapi/build.xml

          Show
          Rick Hillegas added a comment - Attaching derby-4845-02-aa-flipUtilsAndTypes.diff. This patch fixes a build dependency problem which surfaced when I sync'd with the head of the trunk. Committed at subversion revision 1031174. I think that the problem was caused by some submission which made the compilation of ...iapi.util start compiling what's in iapi.types. That latter directory has a special build target which compiles SqlXmlUtil with a special classpath which includes xercesImpl.jar. You can't just compile the contents of iapi.types with the ordinary jdk1.4 classpath. The solution was to flip the order in which we compile ...iapi.util and ...iapi.types. Now we compile ...iapi.types first. Hopefully this patch doesn't destabilize someone else's build. Touches the following file: M java/engine/org/apache/derby/iapi/build.xml
          Hide
          Knut Anders Hatlen added a comment -

          One improvement we could make, is changing the ant javac targets so that they don't compile java files that are not explicitly added to the list of files to compile. The way to do this is described here: http://ant.apache.org/manual/Tasks/javac.html#srcdirnote (basically, just add sourcepath="" to the javac targets). If we take that approach, the build will fail when it comes across a dependency that we haven't specified in the build script.

          Show
          Knut Anders Hatlen added a comment - One improvement we could make, is changing the ant javac targets so that they don't compile java files that are not explicitly added to the list of files to compile. The way to do this is described here: http://ant.apache.org/manual/Tasks/javac.html#srcdirnote (basically, just add sourcepath="" to the javac targets). If we take that approach, the build will fail when it comes across a dependency that we haven't specified in the build script.
          Hide
          Knut Anders Hatlen added a comment -

          One thing that's causing problems for the dependency tracking is that we compile the impl classes in the engine before we build the iapi classes. Since the impl classes depend on the iapi classes, most of the iapi classes will already have been built when the iapi build target is invoked. Building iapi before impl is the right way to do it, but that doesn't work currently because of some ugly dependencies from iapi to impl that have snuck in and cause some impl classes to be built using the wrong classpath if built that way.

          It may be a good, incremental improvement if we remove the dependencies from iapi to impl, and then make iapi being built before impl. Once that's done, we can set sourcepath="" on the iapi target (see previous comment) to prevent accidental reintroduction of dependencies on impl classes.

          The iapi classes currently have many dependencies on implementation classes. Here are three patches that remove some of the dependencies:

          • d4585-iapi-statementnode.diff:

          The implementation class StatementNode is used in the signatures of methods in the TriggerDescriptor and DataDictionary iapi interfaces. The patch changes the signatures to use the iapi interface Visitable instead of StatementNode. In some cases that's specific enough. In other cases, the impl classes that use these methods need to cast the return value to StatementNode now. Actually, most callers of the methods already cast the return value to StatementNode (which suggests that the a less specific interface has been used in the signature before too) or to sub-classes of StatementNode, and didn't need to be changed.

          • d4845-iapi-resultsetstatistics.diff:

          To apply this patch, first run: svn mv java/engine/org/apache/derby/impl/sql/execute/rts/ResultSetStatistics.java java/engine/org/apache/derby/iapi/sql/execute/ResultSetStatistics.java

          The interface ResultSetStatistics lives in the impl sub-tree. However, it is used in the signature of the iapi interface ResultSetStatisticsFactory. It is also the parent of the iapi interface XPLAINVisitor. This suggests that the interface really belongs in iapi and not in impl. The patch moves the interface to iapi and updates import statements to point to the new location.

          d4845-iapi-dependablefinder.diff:

          The impl classes DDdependableFinder and DDColumnDependableFinder are instantiated directly by the iapi class TupleDescriptor and many of its sub-classes. Since the DD* classes are specific to the data dictionary implementation, the patch adds creator methods to the DataDictionary interface and moves the actual creation of these instances into DataDictionaryImpl. Then we don't need to reference these two implementation classes from iapi classes.

          All the regression tests ran cleanly with these three patches.

          Show
          Knut Anders Hatlen added a comment - One thing that's causing problems for the dependency tracking is that we compile the impl classes in the engine before we build the iapi classes. Since the impl classes depend on the iapi classes, most of the iapi classes will already have been built when the iapi build target is invoked. Building iapi before impl is the right way to do it, but that doesn't work currently because of some ugly dependencies from iapi to impl that have snuck in and cause some impl classes to be built using the wrong classpath if built that way. It may be a good, incremental improvement if we remove the dependencies from iapi to impl, and then make iapi being built before impl. Once that's done, we can set sourcepath="" on the iapi target (see previous comment) to prevent accidental reintroduction of dependencies on impl classes. The iapi classes currently have many dependencies on implementation classes. Here are three patches that remove some of the dependencies: d4585-iapi-statementnode.diff: The implementation class StatementNode is used in the signatures of methods in the TriggerDescriptor and DataDictionary iapi interfaces. The patch changes the signatures to use the iapi interface Visitable instead of StatementNode. In some cases that's specific enough. In other cases, the impl classes that use these methods need to cast the return value to StatementNode now. Actually, most callers of the methods already cast the return value to StatementNode (which suggests that the a less specific interface has been used in the signature before too) or to sub-classes of StatementNode, and didn't need to be changed. d4845-iapi-resultsetstatistics.diff: To apply this patch, first run: svn mv java/engine/org/apache/derby/impl/sql/execute/rts/ResultSetStatistics.java java/engine/org/apache/derby/iapi/sql/execute/ResultSetStatistics.java The interface ResultSetStatistics lives in the impl sub-tree. However, it is used in the signature of the iapi interface ResultSetStatisticsFactory. It is also the parent of the iapi interface XPLAINVisitor. This suggests that the interface really belongs in iapi and not in impl. The patch moves the interface to iapi and updates import statements to point to the new location. d4845-iapi-dependablefinder.diff: The impl classes DDdependableFinder and DDColumnDependableFinder are instantiated directly by the iapi class TupleDescriptor and many of its sub-classes. Since the DD* classes are specific to the data dictionary implementation, the patch adds creator methods to the DataDictionary interface and moves the actual creation of these instances into DataDictionaryImpl. Then we don't need to reference these two implementation classes from iapi classes. All the regression tests ran cleanly with these three patches.
          Hide
          Knut Anders Hatlen added a comment -

          Committed d4845-iapi-resultsetstatistics.diff with revision 1196542.
          Committed d4845-iapi-statementnode.diff with revision 1196560.
          Committed d4845-iapi-dependablefinder.diff with revision 1196562.

          Show
          Knut Anders Hatlen added a comment - Committed d4845-iapi-resultsetstatistics.diff with revision 1196542. Committed d4845-iapi-statementnode.diff with revision 1196560. Committed d4845-iapi-dependablefinder.diff with revision 1196562.
          Hide
          Knut Anders Hatlen added a comment -

          The NodeFactory class depends on the implementation classes QueryTreeNode, ResultColumnList and ResultSetNode. There's even a comment saying that these classes shouldn't have been used there:

          /* Strictly speaking we shouldn't import classes under the impl hierarchy here

          • but this is work in progress.
          • manish - Wed Mar 28 13:05:19 PST 2001
            */
            import org.apache.derby.impl.sql.compile.QueryTreeNode;
            import org.apache.derby.impl.sql.compile.ResultColumnList;
            import org.apache.derby.impl.sql.compile.ResultSetNode;

          The proper fix is to get rid of NodeFactory completely (see DERBY-673), but that's a bigger task. For now, we can make some smaller changes to remove its dependencies on impl classes.

          The attached patch (d4845-iapi-slimmer-nodefactory.diff) is a first step. It moves the method mapTableAsVTI() into FromBaseTable, the only class that ever calls that method. Similarly, it moves getCreateAliasNode() into the parser (according to the code comments, it was carved out of the parser in order to support ALTER PUBLICATION, which isn't supported by Derby).

          By moving these methods out of NodeFactory and into the impl classes that use them, the dependencies on ResultColumnList and ResultSetNode go away.

          All the regression tests passed with the patch.

          (One small change in the patch that may look unrelated, is the signature change for MethodCallNode.addParms(). Now it accepts java.util.List instead of java.util.Vector. This was done so that we didn't have to move the emptyVector field from NodeFactory to FromBaseTable. With the signature change, we could use Collections.EMPTY_LIST instead, and simply remove the emptyVector field from NodeFactory instead of moving it into FromBaseTable.)

          Show
          Knut Anders Hatlen added a comment - The NodeFactory class depends on the implementation classes QueryTreeNode, ResultColumnList and ResultSetNode. There's even a comment saying that these classes shouldn't have been used there: /* Strictly speaking we shouldn't import classes under the impl hierarchy here but this is work in progress. manish - Wed Mar 28 13:05:19 PST 2001 */ import org.apache.derby.impl.sql.compile.QueryTreeNode; import org.apache.derby.impl.sql.compile.ResultColumnList; import org.apache.derby.impl.sql.compile.ResultSetNode; The proper fix is to get rid of NodeFactory completely (see DERBY-673 ), but that's a bigger task. For now, we can make some smaller changes to remove its dependencies on impl classes. The attached patch (d4845-iapi-slimmer-nodefactory.diff) is a first step. It moves the method mapTableAsVTI() into FromBaseTable, the only class that ever calls that method. Similarly, it moves getCreateAliasNode() into the parser (according to the code comments, it was carved out of the parser in order to support ALTER PUBLICATION, which isn't supported by Derby). By moving these methods out of NodeFactory and into the impl classes that use them, the dependencies on ResultColumnList and ResultSetNode go away. All the regression tests passed with the patch. (One small change in the patch that may look unrelated, is the signature change for MethodCallNode.addParms(). Now it accepts java.util.List instead of java.util.Vector. This was done so that we didn't have to move the emptyVector field from NodeFactory to FromBaseTable. With the signature change, we could use Collections.EMPTY_LIST instead, and simply remove the emptyVector field from NodeFactory instead of moving it into FromBaseTable.)
          Hide
          Knut Anders Hatlen added a comment -

          Committed d4845-iapi-slimmer-nodefactory.diff with revision 1198695.

          Show
          Knut Anders Hatlen added a comment - Committed d4845-iapi-slimmer-nodefactory.diff with revision 1198695.
          Hide
          Knut Anders Hatlen added a comment -

          The previous commit removed references to ResultColumnList and ResultSetNode from NodeFactory, but it still references the impl class QueryTreeNode, which is the return type for the getNode() methods.

          To avoid exposing the impl class through iapi, I've added a Node interface that QueryTreeNode implements and made the getNode() methods return Node instead of QueryTreeNode. See the attached d4845-iapi-nodefactory-getnode.diff patch.

          All regression tests ran cleanly with the patch.

          Show
          Knut Anders Hatlen added a comment - The previous commit removed references to ResultColumnList and ResultSetNode from NodeFactory, but it still references the impl class QueryTreeNode, which is the return type for the getNode() methods. To avoid exposing the impl class through iapi, I've added a Node interface that QueryTreeNode implements and made the getNode() methods return Node instead of QueryTreeNode. See the attached d4845-iapi-nodefactory-getnode.diff patch. All regression tests ran cleanly with the patch.
          Hide
          Knut Anders Hatlen added a comment -

          Committed revision 1199160.

          Show
          Knut Anders Hatlen added a comment - Committed revision 1199160.
          Hide
          Knut Anders Hatlen added a comment -

          Attaching patch d4845-iapi-unused-imports.diff which removes unused imports in some of the iapi classes. The unused imports included classes in the impl package and caused unnecessary compile-time dependencies on impl from the iapi package.

          Committed revision 1199348.

          Show
          Knut Anders Hatlen added a comment - Attaching patch d4845-iapi-unused-imports.diff which removes unused imports in some of the iapi classes. The unused imports included classes in the impl package and caused unnecessary compile-time dependencies on impl from the iapi package. Committed revision 1199348.
          Hide
          Knut Anders Hatlen added a comment -

          DataDictionaryImpl has a public boolean field called readOnlyUpgrade. This field is accessed directly, also from iapi code which needs to cast the data dictionary instance to DataDictionaryImpl.

          The attached patch d4845-iapi-readOnlyUpgrade.diff adds a method called isReadOnlyUpgrade() to the DataDictionary interface so that we don't need to cast the instance to the implementation class in order to access the field.

          All the regression tests ran cleanly with the patch.

          Show
          Knut Anders Hatlen added a comment - DataDictionaryImpl has a public boolean field called readOnlyUpgrade. This field is accessed directly, also from iapi code which needs to cast the data dictionary instance to DataDictionaryImpl. The attached patch d4845-iapi-readOnlyUpgrade.diff adds a method called isReadOnlyUpgrade() to the DataDictionary interface so that we don't need to cast the instance to the implementation class in order to access the field. All the regression tests ran cleanly with the patch.
          Hide
          Knut Anders Hatlen added a comment -

          Committed d4845-iapi-readOnlyUpgrade.diff with revision 1200232.

          Show
          Knut Anders Hatlen added a comment - Committed d4845-iapi-readOnlyUpgrade.diff with revision 1200232.
          Hide
          Knut Anders Hatlen added a comment -

          The previous commit broke the build for those who have
          jsr169compile.classpath configured:
          http://mail-archives.apache.org/mod_mbox/db-derby-dev/201111.mbox/%3C20111110092611.21938.qmail%40tyr72%3E

          The reason seems to be that dependencies on Java SE has sneaked into
          code that's supposed to work on CDC/Foundation Profile. It didn't use
          to fail at compile time before because the classes were compiled by a
          different target than intended. When we removed some implicit build
          dependencies in the previous patch, these classes ended up being
          compiled using the intended target, which used a stricter compile
          classpath than the implicit target used before.

          This problem could also be seen before the previous commit, by
          touching java/engine/org/apache/derby/impl/db/SlaveDatabase.java and
          doing an incremental build on an otherwise fully compiled source tree.
          This is exactly the kind of problems we want to fix in this JIRA
          issue.

          I think this isn't just a build problem, but also a run-time problem
          on CDC/FP. For example, the following code raises a
          java.lang.NoClassDefFoundError with Derby 10.8.2.2 on Oracle Java ME
          Embedded Client:

          EmbeddedSimpleDataSource ds = new EmbeddedSimpleDataSource();
          ds.setDatabaseName("wombat");
          ds.setCreateDatabase("create");
          ds.getConnection().close();

          ds.setCreateDatabase(null);
          ds.setDatabaseName(null);
          ds.setShutdownDatabase("shutdown");
          ds.setConnectionAttributes("deregister=true");
          ds.getConnection();

          When the attached patch is applied, it changes to the expected
          exception: java.sql.SQLException: Derby system shutdown.

          Here's a description of the changes in d4845-buildbreak.diff:

          • java/engine/org/apache/derby/impl/db/SlaveDatabase.java:

          This class is supposed to be compiled against CDC/FP libraries (and
          now it actually is), but it references java.sql.Driver and
          java.sql.DriverManager, which are not available on that platform. The
          patch changes the code to use InternalDriver to shut down the
          database. (The original code didn't cause any run-time failures on
          CDC/FP because it the code is wrapped in a try/catch that ignores all
          exceptions.)

          • java/engine/org/apache/derby/iapi/reference/Limits.java
          • java/engine/org/apache/derby/impl/jdbc/LOBStoredProcedure.java
          • java/engine/org/apache/derby/impl/sql/catalog/DataDictionaryImpl.java

          DataDictionaryImpl imported LOBStoredProcedure in order to get a
          constant. Since LOBStoredProcedure is not supposed to work on CDC/FP,
          whereas DataDictionaryImpl is, DataDictionaryImpl shouldn't reference
          it directly. The patch moves the constants to the Limits interface so
          that DataDictionaryImpl doesn't have to import LOBStoredProcedure.

          • java/engine/org/apache/derby/jdbc/AutoloadedDriver.java
          • java/engine/org/apache/derby/jdbc/InternalDriver.java

          InternalDriver calls AutoloadedDriver.setDeregister(), but it should
          not reference code that only works on Java SE, since it is supposed to
          work on CDC/FP. The patch moves the methods setDeregister() and
          getDeregister() to InternalDriver and lets AutoloadedDriver fetch this
          info from InternalDriver instead.

          The build works again with jsr169compile.classpath set when the patch
          is applied. No problems were found when running suites.All and
          derbyall.

          Committed revision 1200293.

          Show
          Knut Anders Hatlen added a comment - The previous commit broke the build for those who have jsr169compile.classpath configured: http://mail-archives.apache.org/mod_mbox/db-derby-dev/201111.mbox/%3C20111110092611.21938.qmail%40tyr72%3E The reason seems to be that dependencies on Java SE has sneaked into code that's supposed to work on CDC/Foundation Profile. It didn't use to fail at compile time before because the classes were compiled by a different target than intended. When we removed some implicit build dependencies in the previous patch, these classes ended up being compiled using the intended target, which used a stricter compile classpath than the implicit target used before. This problem could also be seen before the previous commit, by touching java/engine/org/apache/derby/impl/db/SlaveDatabase.java and doing an incremental build on an otherwise fully compiled source tree. This is exactly the kind of problems we want to fix in this JIRA issue. I think this isn't just a build problem, but also a run-time problem on CDC/FP. For example, the following code raises a java.lang.NoClassDefFoundError with Derby 10.8.2.2 on Oracle Java ME Embedded Client: EmbeddedSimpleDataSource ds = new EmbeddedSimpleDataSource(); ds.setDatabaseName("wombat"); ds.setCreateDatabase("create"); ds.getConnection().close(); ds.setCreateDatabase(null); ds.setDatabaseName(null); ds.setShutdownDatabase("shutdown"); ds.setConnectionAttributes("deregister=true"); ds.getConnection(); When the attached patch is applied, it changes to the expected exception: java.sql.SQLException: Derby system shutdown. Here's a description of the changes in d4845-buildbreak.diff: java/engine/org/apache/derby/impl/db/SlaveDatabase.java: This class is supposed to be compiled against CDC/FP libraries (and now it actually is), but it references java.sql.Driver and java.sql.DriverManager, which are not available on that platform. The patch changes the code to use InternalDriver to shut down the database. (The original code didn't cause any run-time failures on CDC/FP because it the code is wrapped in a try/catch that ignores all exceptions.) java/engine/org/apache/derby/iapi/reference/Limits.java java/engine/org/apache/derby/impl/jdbc/LOBStoredProcedure.java java/engine/org/apache/derby/impl/sql/catalog/DataDictionaryImpl.java DataDictionaryImpl imported LOBStoredProcedure in order to get a constant. Since LOBStoredProcedure is not supposed to work on CDC/FP, whereas DataDictionaryImpl is, DataDictionaryImpl shouldn't reference it directly. The patch moves the constants to the Limits interface so that DataDictionaryImpl doesn't have to import LOBStoredProcedure. java/engine/org/apache/derby/jdbc/AutoloadedDriver.java java/engine/org/apache/derby/jdbc/InternalDriver.java InternalDriver calls AutoloadedDriver.setDeregister(), but it should not reference code that only works on Java SE, since it is supposed to work on CDC/FP. The patch moves the methods setDeregister() and getDeregister() to InternalDriver and lets AutoloadedDriver fetch this info from InternalDriver instead. The build works again with jsr169compile.classpath set when the patch is applied. No problems were found when running suites.All and derbyall. Committed revision 1200293.
          Hide
          Knut Anders Hatlen added a comment -

          The impl.jdbc.Util class is used by many of the brokered JDBC objects in the iapi.jdbc package. The attached patch, d4845-iapi-stmt-isClosed.diff, removes one usage in BrokeredStatement.isClosed(). That method currently throws a not implemented exception and uses the Util class to generate the exception. The patch instead implements the method in BrokeredStatement and removes the more or less identical overrides in BrokeredStatement40, BrokeredPreparedStatement40 and BrokeredCallableStatement40.

          All the regression tests ran cleanly with the patch.

          Committed revision 1204934.

          Show
          Knut Anders Hatlen added a comment - The impl.jdbc.Util class is used by many of the brokered JDBC objects in the iapi.jdbc package. The attached patch, d4845-iapi-stmt-isClosed.diff, removes one usage in BrokeredStatement.isClosed(). That method currently throws a not implemented exception and uses the Util class to generate the exception. The patch instead implements the method in BrokeredStatement and removes the more or less identical overrides in BrokeredStatement40, BrokeredPreparedStatement40 and BrokeredCallableStatement40. All the regression tests ran cleanly with the patch. Committed revision 1204934.
          Hide
          Knut Anders Hatlen added a comment -

          BrokeredConnection40 imports EmbedConnection40 in order to be able to call some JDBC 4.1 methods. Typically, we would have added the methods to the EngineConnection interface so that we didn't have to reference the implementation class directly. The problem with these methods was that they had java.util.concurrent.Executor in their signatures, so they couldn't be put into the EngineConnection interface (because it has to be compilable against the CDC/FP libraries, which don't have the Executor interface).

          The attached patch, d4845-iapi-brokered-connection.diff, adds a new interface (EngineConnection40) which contains these methods. BrokeredConnection40 and EmbedConnection40 implement the interface. This makes it possible to access the methods from BrokeredConnection40 without casting the underlying connection instance to the implementation class EmbedConnection40.

          All the regression tests ran cleanly with the patch.

          Show
          Knut Anders Hatlen added a comment - BrokeredConnection40 imports EmbedConnection40 in order to be able to call some JDBC 4.1 methods. Typically, we would have added the methods to the EngineConnection interface so that we didn't have to reference the implementation class directly. The problem with these methods was that they had java.util.concurrent.Executor in their signatures, so they couldn't be put into the EngineConnection interface (because it has to be compilable against the CDC/FP libraries, which don't have the Executor interface). The attached patch, d4845-iapi-brokered-connection.diff, adds a new interface (EngineConnection40) which contains these methods. BrokeredConnection40 and EmbedConnection40 implement the interface. This makes it possible to access the methods from BrokeredConnection40 without casting the underlying connection instance to the implementation class EmbedConnection40. All the regression tests ran cleanly with the patch.
          Hide
          Knut Anders Hatlen added a comment -

          Committed d4845-iapi-brokered-connection.diff to trunk with revision 1205195.

          Attaching another patch, d4845-iapi-brokered-callable-stmt.diff, which fixes a similar dependency in BrokeredCallableStatement40 by introducing an EngineCallableStatement40 interface and making EmbedCallableStatement40 implement that interface. All regression tests ran cleanly.

          Show
          Knut Anders Hatlen added a comment - Committed d4845-iapi-brokered-connection.diff to trunk with revision 1205195. Attaching another patch, d4845-iapi-brokered-callable-stmt.diff, which fixes a similar dependency in BrokeredCallableStatement40 by introducing an EngineCallableStatement40 interface and making EmbedCallableStatement40 implement that interface. All regression tests ran cleanly.
          Hide
          Knut Anders Hatlen added a comment -

          Committed d4845-iapi-brokered-callable-stmt.diff to trunk with revision 1205378.

          Show
          Knut Anders Hatlen added a comment - Committed d4845-iapi-brokered-callable-stmt.diff to trunk with revision 1205378.
          Hide
          Knut Anders Hatlen added a comment -

          As mentioned in an earlier comment, the impl.jdbc.Util class is used by many of the brokered JDBC objects in the iapi.jdbc package to generate SQLExceptions.

          The attached patch, d4845-iapi-exception-util.diff, makes them stop using the Util class. This is done by creating an iapi interface that SQLExceptionFactory implements, and accessing SQLExceptionFactory through that interface instead of through the Util class.

          All the regression tests passed with the patch.

          Show
          Knut Anders Hatlen added a comment - As mentioned in an earlier comment, the impl.jdbc.Util class is used by many of the brokered JDBC objects in the iapi.jdbc package to generate SQLExceptions. The attached patch, d4845-iapi-exception-util.diff, makes them stop using the Util class. This is done by creating an iapi interface that SQLExceptionFactory implements, and accessing SQLExceptionFactory through that interface instead of through the Util class. All the regression tests passed with the patch.
          Hide
          Knut Anders Hatlen added a comment -

          Committed d4845-iapi-exception-util.diff to trunk with revision 1205753.

          Show
          Knut Anders Hatlen added a comment - Committed d4845-iapi-exception-util.diff to trunk with revision 1205753.
          Hide
          Knut Anders Hatlen added a comment -

          d4845-jsr169-nonoptional.diff addresses another issue in the build scripts. Earlier, the JSR-169 specific parts were optional, but now they're always built. However, the build scripts still model the old world, by compiling the mandatory parts first and the (previously) optional parts last. This adds to the complexity of the scripts, and also causes some redundancies (because some of the previously optional targets are now dependencies of the mandatory targets, but the optional targets are still invoked explicitly later in the build process).

          The patch merges the optional targets with the non-optional ones, so that they get compiled in one go. It also removes the target that rewrites modules.properties for J2ME support, since that information could now be hard-coded in modules.properties.

          I ran the regression tests successfully on both Java 6 and phoneME.

          Show
          Knut Anders Hatlen added a comment - d4845-jsr169-nonoptional.diff addresses another issue in the build scripts. Earlier, the JSR-169 specific parts were optional, but now they're always built. However, the build scripts still model the old world, by compiling the mandatory parts first and the (previously) optional parts last. This adds to the complexity of the scripts, and also causes some redundancies (because some of the previously optional targets are now dependencies of the mandatory targets, but the optional targets are still invoked explicitly later in the build process). The patch merges the optional targets with the non-optional ones, so that they get compiled in one go. It also removes the target that rewrites modules.properties for J2ME support, since that information could now be hard-coded in modules.properties. I ran the regression tests successfully on both Java 6 and phoneME.
          Hide
          Knut Anders Hatlen added a comment -

          Committed d4845-jsr169-nonoptional.diff with revision 1222151.

          Show
          Knut Anders Hatlen added a comment - Committed d4845-jsr169-nonoptional.diff with revision 1222151.

            People

            • Assignee:
              Unassigned
              Reporter:
              Rick Hillegas
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:

                Development