On 2. 8. 2026 15:07, Daniel Sahlberg wrote:
Den ons 29 juli 2026 kl 12:52 skrev Branko Čibej <[email protected]>:

    On 29. 7. 2026 10:58, Daniel Sahlberg wrote:


    Den ons 29 juli 2026 kl 04:32 skrev Jun Omae <[email protected]>:

        On 2026/07/28 23:58, Daniel Sahlberg wrote:
        > Den mån 27 juli 2026 kl 00:09 skrev Jun Omae
        <[email protected] <mailto:[email protected]>>:
        >
        >     > Is there a problem if we include the test classes?
        >
        >     No direct issues have been found so far. However,
        static initializers within test classes are executed simply
        by loading the class.
        >
        >     [[[
        >     $ grep -r 'static *{' subversion/bindings/javahl
        >
         
subversion/bindings/javahl/tests/org/apache/subversion/javahl/UtilTests.java:
           static {
        >
         
subversion/bindings/javahl/tests/org/apache/subversion/javahl/UtilTests.java:
           static {
        >     ]]]
        >
        >
        > Thank you!
        >
        > I think I got it working on 1.14.x, successfully created a
        JAR file without the tests.
        >
        > It still seems to fail on trunk. Can you do the same steps
        above on trunk?
        >
        > Cheers,
        > Daniel
        >

        Hm, I get the same failures on trunk.
        It seems that the changes of the behavior is introduced in
        r1933892.
        At least, ./configure should stop and/or warn it if the junit
        is NOT optional when javahl enabled.

        [[[
        $ /bin/sh autogen.sh
        $ ./configure --prefix=/dev/shm/svn/trunk --without-apxs
        --without-swig --without-swig-{perl,python,ruby}
        --enable-javahl --with-jdk=/usr/lib/jvm/java-11-openjdk-amd64
        --without-junit PYTHON=/usr/bin/python3
        $ make -j$(nproc) all
        $ make javahl

        $ find subversion/bindings/javahl -name '*.class' | wc -l
        277
        $ find subversion/bindings/javahl -name '*Test*.class' | wc -l
        0    #==> tests/**/*.java are compiled yet.

        $ make install
        $ make install-javahl    # Why this task builds tests/**/*.java?
        ...
        
/dev/shm/subversion-trunk/subversion/bindings/javahl/tests/org/tigris/subversion/javahl/BasicTests.java:1708:
        error: cannot find symbol
                assertEquals("wrong revision from commit",
                ^
          symbol:   method assertEquals(String,long,int)
          location: class BasicTests
        
/dev/shm/subversion-trunk/subversion/bindings/javahl/tests/org/tigris/subversion/javahl/BasicTests.java:1756:
        error: cannot find symbol
                assertEquals("wrong revision from update",
                ^
          symbol:   method assertEquals(String,long,int)
          location: class BasicTests
        Note: Some input files use or override a deprecated API.
        Note: Recompile with -Xlint:deprecation for details.
        100 errors
        ]]]


    Thanks for confirming! It seems like 1.15.x is also affected by
    this (I'll bring it up on the release thread).

    To add three more data points to the discussion, I checked Fedora
    43, 44 and FreeBSD 14. The tests are present in Fedora's
    svn-javahl.jar but not in FreeBSD's svn-javahl.jar. Obviously
    different distributors use different options when building. That
    is unfortunate since we are distributing different ABIs.

    I think it would be cleaner to have the tests in
    svn-javahl-tests.jar if built. Maybe we want to install this
    alongside svn-javahl.jar. That way Fedora and Ubuntu can keep
    distributing the tests and FreeBSD not, while everyone's
    svn-javahl.jar is the same.

    Does that make sense?


    The following makes sense:

    1. There is no regression in autotools. It is what it is. Note
    that putting test classes in a separate jar has wider consequences
    than just the ABI. For example, JavaHL tests would fail in
    autotools builds. We'd have to change the way the classpath is
    constructed in `make check-*-javahl`.

    2. No change is needed in 1.15.x. Autotools are consistent with
    earlier releases and CMake doesn't build JavaHL. IFF we want to
    change this in CMake for 1.15, then we have further questions to
    consider:

        2.1. CMake and Autotools/vcxproj should produce identical
        results given identical options. In this case specifically I
        mean that the CMake build should create the same lib/dll names
        and the same JavaHL jar (singular) as the vcxproj generator.


    3. Regardless of the answer to 2: If we decide to not include test
    classes in svn-javhl.jar, then we have to consider how to mitigate
    the ABI change.

    Personally I'd prefer changing the build requirements to changing
    the ABI in svn-javahl.jar. That means that --enable-javahl implies
    that JUnit is mandatory.

    -- Brane


Is the patch below an acceptable way of requiring --with-junit?

Cheers,
Daniel


 [[[
Index: configure.ac <http://configure.ac>
===================================================================
--- configure.ac <http://configure.ac>        (revision 1936330)
+++ configure.ac <http://configure.ac>        (working copy)
@@ -1748,6 +1748,7 @@

 dnl Possibly compile JavaHL
 do_javahl_build=no
+has_junit=no

This initialisation is not needed.

 AC_ARG_ENABLE(javahl,
   AS_HELP_STRING([--enable-javahl],
              [Enable compilation of Java high-level bindings (requires C++)]),
@@ -1755,6 +1756,21 @@
         do_javahl_build="yes"
     fi
   ])
+AC_ARG_WITH(junit,
+AS_HELP_STRING([--with-junit=PATH],
+               [Specify a path to the junit JAR file.]),
+[
+    if test "$withval" != "no"; then
+      if test -n "$JAVA_CLASSPATH"; then
+        JAVA_CLASSPATH="$withval:$JAVA_CLASSPATH"
+      else
+        JAVA_CLASSPATH="$withval"
+      fi
+      JAVAHL_TESTS_TARGET="javahl-tests"
+      JAVAHL_COMPAT_TESTS_TARGET="javahl-compat-tests"
+      has_junit="yes"

Should be;
   have_junit="yes"

+    fi
+])

 JAVAHL_OBJDIR=""
 INSTALL_EXTRA_JAVAHL_LIB=""
@@ -1763,6 +1779,12 @@
 JAVAHL_COMPAT_TESTS_TARGET=""
 LT_CXX_LIBADD=""
 if test "$do_javahl_build" = "yes"; then
+  dnl Since r1933892 junit is required to build the JavaHL bindings
+  if test "$has_junit" = "no"; then

And here:

  if test "$have_junit" != "yes"; then


The idea is to set the have_junit variable exactly once. If it's not set, "$have_junit" will be an empty string.

+  AC_MSG_ERROR([--with-junit must be specified if building the Java high-level
+                  bindings.])
+  fi
+
   dnl Check for suitable JDK
   if test "$JDK_SUITABLE" = "no"; then
     AC_MSG_ERROR([Cannot compile JavaHL without a suitable JDK.
@@ -1802,21 +1824,6 @@
 AC_SUBST(JAVAHL_OBJDIR)
 AC_SUBST(FIX_JAVAHL_LIB)
 AC_SUBST(LT_CXX_LIBADD)
-
-AC_ARG_WITH(junit,
-AS_HELP_STRING([--with-junit=PATH],
-               [Specify a path to the junit JAR file.]),
-[
-    if test "$withval" != "no"; then
-      if test -n "$JAVA_CLASSPATH"; then
-        JAVA_CLASSPATH="$withval:$JAVA_CLASSPATH"
-      else
-        JAVA_CLASSPATH="$withval"
-      fi
-      JAVAHL_TESTS_TARGET="javahl-tests"
-      JAVAHL_COMPAT_TESTS_TARGET="javahl-compat-tests"
-    fi
-])
 AC_SUBST(JAVA_CLASSPATH)
 AC_SUBST(JAVAHL_TESTS_TARGET)
 AC_SUBST(JAVAHL_COMPAT_TESTS_TARGET)
]]]

Reply via email to